Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/qa-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ jobs:
cp firewall-java/.github/workflows/Dockerfile.qa zen-demo-java/Dockerfile

- name: Run Firewall QA Tests
uses: AikidoSec/firewall-tester-action@v1.0.0
uses: AikidoSec/firewall-tester-action@v1.0.12
with:
dockerfile_path: ./zen-demo-java/Dockerfile
app_port: 8080
sleep_before_test: 30
skip_tests: test_ssrf,test_demo_apps_generic_tests
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.lang.reflect.Executable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.IDN;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URL;
Expand All @@ -32,6 +33,29 @@ public static class InetAdvice {
// Since we have to wrap a native Java Class stuff gets more complicated
// The classpath is not the same anymore, and we can't import our modules directly.
// To bypass this issue we load collectors from a .jar file

// Java's system resolver rejects non-ASCII hostnames, so convert IDN to Punycode
// before the real lookup runs. Without this, getAllByName throws UnknownHostException
// and OnMethodExit never fires β€” meaning DNSRecordCollector can't block or track
// the hostname.
@Advice.OnMethodEnter(suppress = Throwable.class)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting and replacing the original 'hostname' argument (IDN.toASCII) before lookup can obscure the original domain and enable bypasses. Preserve or log the original hostname (or avoid in-place mutation) so downstream checks see the true input.

Details

✨ AI Reasoning
​The change adds an entry advice that converts non-ASCII hostnames to ASCII (Punycode) before DNS lookup. Transforming inputs prior to resolution can hide the original domain representation from later inspection (e.g., logs or collectors) and may be used to bypass domain-based detection or allowlist checks. The transformation is done silently (catching IllegalArgumentException and returning) and replaces the hostname argument in place, which can obscure the original value from downstream code. This behavior changes how hostnames are presented to the rest of the system and therefore can degrade transparency and enable hiding malicious domains.

πŸ”§ How do I fix it?
Ensure code is transparent and not intentionally obfuscated. Avoid hiding functionality from code review. Focus on intent and deception, not specific patterns.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

public static void before(
@Advice.Argument(value = 0, readOnly = false) String hostname
) {
if (hostname == null) {
return;
}
for (int i = 0; i < hostname.length(); i++) {
if (hostname.charAt(i) > 0x7F) {
try {
hostname = IDN.toASCII(hostname, IDN.ALLOW_UNASSIGNED);
} catch (IllegalArgumentException ignored) {
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty catch of IllegalArgumentException in the hostname normalization block silently swallows errors; add logging or explicit handling so failures are observable.

Suggested change
} catch (IllegalArgumentException ignored) {
} catch (IllegalArgumentException e) {
// IDN conversion failed; log and continue with original hostname
System.out.println("AIKIDO: Failed to convert hostname to ASCII: " + e.getMessage());
Details

✨ AI Reasoning
​A new OnMethodEnter wrapper was added to convert internationalized hostnames to ASCII. Inside that addition, an empty catch block catches IllegalArgumentException and does nothing. Silently swallowing this exception hides parsing/conversion failures and makes debugging harder. The catch contains no handling, logging, or comment explaining intent; it neither rethrows nor records the error. This harms maintainability and can obscure failures in hostname normalization during DNS resolution.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

}
return;
}
}
}

@Advice.OnMethodExit
public static void after(
@Advice.Argument(0) String hostname,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dev.aikido.agent_api.collectors;

import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.storage.BypassedContextStore;
import dev.aikido.agent_api.storage.HostnamesStore;
import dev.aikido.agent_api.storage.PendingHostnamesStore;
import dev.aikido.agent_api.storage.ServiceConfigStore;
Expand Down Expand Up @@ -34,20 +35,24 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
// store stats
StatisticsStore.registerCall("java.net.InetAddress.getAllByName", OperationKind.OUTGOING_HTTP_OP);

boolean bypassed = BypassedContextStore.isBypassed();

// Consume pending ports recorded by URLCollector for this hostname.
// Removing them here ensures each (hostname, port) pair is counted exactly once.
Set<Integer> ports = PendingHostnamesStore.getAndRemove(hostname);
if (!ports.isEmpty()) {
for (int port : ports) {
HostnamesStore.incrementHits(hostname, port);
if (!bypassed) {
// Bypassed IPs are trusted β€” don't report their outbound hostnames in heartbeats.
if (!ports.isEmpty()) {
for (int port : ports) {
HostnamesStore.incrementHits(hostname, port);
}
} else {
HostnamesStore.incrementHits(hostname, 0);
}
Comment on lines +43 to 51
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if (!bypassed) block in report adds nesting around the main hostname-hit logic; invert the condition or use an early return/guard to flatten the method for clarity.

Show fix
Suggested change
if (!bypassed) {
// Bypassed IPs are trusted β€” don't report their outbound hostnames in heartbeats.
if (!ports.isEmpty()) {
for (int port : ports) {
HostnamesStore.incrementHits(hostname, port);
}
} else {
HostnamesStore.incrementHits(hostname, 0);
}
if (bypassed) {
// Bypassed IPs are trusted β€” don't report their outbound hostnames in heartbeats.
} else if (!ports.isEmpty()) {
for (int port : ports) {
HostnamesStore.incrementHits(hostname, port);
}
} else {
HostnamesStore.incrementHits(hostname, 0);
Details

✨ AI Reasoning
​The report method now computes a bypassed flag and then wraps hostname hit-incrementing logic inside an if (!bypassed) else branch, increasing nesting. This buries the normal-path behavior and makes the flow harder to follow; using an early return or inverting the condition would flatten the logic and improve readability.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

} else {
// We still need to report a hit to the hostname for outbound domain blocking
HostnamesStore.incrementHits(hostname, 0);
}

// Block if the hostname is in the blocked domains list
if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname)) {
if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname) && !bypassed) {
logger.debug("Blocking DNS lookup for domain: %s", hostname);
throw BlockedOutboundException.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import dev.aikido.agent_api.helpers.logging.LogManager;
import dev.aikido.agent_api.helpers.logging.Logger;
import dev.aikido.agent_api.storage.AttackQueue;
import dev.aikido.agent_api.storage.BypassedContextStore;
import dev.aikido.agent_api.storage.PendingHostnamesStore;
import dev.aikido.agent_api.storage.ServiceConfigStore;
import dev.aikido.agent_api.storage.ServiceConfiguration;
Expand Down Expand Up @@ -44,8 +45,10 @@ public static Res report(ContextObject newContext) {
// Flush pending hostnames on every context change to prevent the store from
// growing unboundedly when a thread is reused across multiple requests.
PendingHostnamesStore.clear();
BypassedContextStore.clear();

if (config.isIpBypassed(newContext.getRemoteAddress())) {
BypassedContextStore.setBypassed(true);
return null; // do not set context when the IP address is bypassed (zen = off)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dev.aikido.agent_api.helpers.extraction;

import dev.aikido.agent_api.helpers.patterns.LooksLikeJWT;
import dev.aikido.agent_api.vulnerabilities.DangerousBodyException;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.*;
Expand All @@ -9,31 +10,35 @@
import static dev.aikido.agent_api.helpers.patterns.PrimitiveType.isPrimitiveType;

public class StringExtractor {
private static final int MAX_DEPTH = 1024;
// Ensures that we don't get recursion :
Set<Object> scanned = new HashSet<>();
public static Map<String, String> extractStringsFromObject(Object obj) {
return new StringExtractor().extractStringsRecursive(obj, new ArrayList<>());
return new StringExtractor().extractStringsRecursive(obj, new ArrayList<>(), 0);
}
private Map<String, String> extractStringsRecursive(Object target, ArrayList<PathBuilder.PathPart> pathToPayload) {
private Map<String, String> extractStringsRecursive(Object target, ArrayList<PathBuilder.PathPart> pathToPayload, int depth) {
if (depth > MAX_DEPTH) {
throw DangerousBodyException.bodyTooDeep();
}
HashMap<String, String> result = new HashMap<>();
if (target == null || scanned.contains(target)) {
return Map.of(); // Do not rescan objects, because this might lead to recursion.
}
scanned.add(target);

if (target instanceof String targetString) {
result.putAll(extractStringsFromString(targetString, pathToPayload));
result.putAll(extractStringsFromString(targetString, pathToPayload, depth));
} else if (target instanceof Collection<?> || target.getClass().isArray()) {
result.putAll(extractStringsFromArray(target, pathToPayload));
result.putAll(extractStringsFromArray(target, pathToPayload, depth));
} else if (target instanceof Map<?, ?> targetMap) {
result.putAll(extractStringsFromMap(targetMap, pathToPayload));
result.putAll(extractStringsFromMap(targetMap, pathToPayload, depth));
} else if (!isPrimitiveType(target)) { // Stop algorithm if it's a primitive type.
result.putAll(extractStringsFromStructure(target, pathToPayload));
result.putAll(extractStringsFromStructure(target, pathToPayload, depth));
}
return result;
}

private Map<String, String> extractStringsFromString(String target, ArrayList<PathBuilder.PathPart> pathToPayload) {
private Map<String, String> extractStringsFromString(String target, ArrayList<PathBuilder.PathPart> pathToPayload, int depth) {
HashMap<String, String> result = new HashMap<>();
result.put(target, buildPathToPayload(pathToPayload));

Expand All @@ -42,7 +47,7 @@ private Map<String, String> extractStringsFromString(String target, ArrayList<Pa
if (jwtResult.success()) {
ArrayList<PathBuilder.PathPart> newPathToPayload = new ArrayList<>(pathToPayload);
newPathToPayload.add(new PathBuilder.PathPart("jwt"));
Map<String, String> resultsFromJWT = extractStringsRecursive(jwtResult.payload(), newPathToPayload);
Map<String, String> resultsFromJWT = extractStringsRecursive(jwtResult.payload(), newPathToPayload, depth + 1);
for (Map.Entry<String, String> entry : resultsFromJWT.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
Expand All @@ -57,27 +62,27 @@ private Map<String, String> extractStringsFromString(String target, ArrayList<Pa
return result;
}

private Map<String, String> extractStringsFromArray(Object target, ArrayList<PathBuilder.PathPart> pathToPayload) {
private Map<String, String> extractStringsFromArray(Object target, ArrayList<PathBuilder.PathPart> pathToPayload, int depth) {
HashMap<String, String> result = new HashMap<>();
if (target instanceof Collection<?> targetCollection) {
int index = 0;
for (Object element : (Collection<?>) targetCollection) {
ArrayList<PathBuilder.PathPart> newPathToPayload = new ArrayList<>(pathToPayload);
newPathToPayload.add(new PathBuilder.PathPart("array", index));
result.putAll(extractStringsRecursive(element, newPathToPayload));
result.putAll(extractStringsRecursive(element, newPathToPayload, depth + 1));
index++;
}
} else if (target instanceof Object[] targetArray) {
for (int i = 0; i < targetArray.length; i++) {
ArrayList<PathBuilder.PathPart> newPathToPayload = new ArrayList<>(pathToPayload);
newPathToPayload.add(new PathBuilder.PathPart("array", i));
result.putAll(extractStringsRecursive(targetArray[i], newPathToPayload));
result.putAll(extractStringsRecursive(targetArray[i], newPathToPayload, depth + 1));
}
}
return result;
}

private Map<String, String> extractStringsFromMap(Map<?, ?> target, ArrayList<PathBuilder.PathPart> pathToPayload) {
private Map<String, String> extractStringsFromMap(Map<?, ?> target, ArrayList<PathBuilder.PathPart> pathToPayload, int depth) {
HashMap<String, String> result = new HashMap<>();
for (Object key : target.keySet()) {
if (key instanceof String stringKey) {
Expand All @@ -89,12 +94,12 @@ private Map<String, String> extractStringsFromMap(Map<?, ?> target, ArrayList<Pa
} else {
newPathToPayload.add(new PathBuilder.PathPart("object", key.toString()));
}
result.putAll(extractStringsRecursive(target.get(key), newPathToPayload));
result.putAll(extractStringsRecursive(target.get(key), newPathToPayload, depth + 1));
}
return result;
}

private Map<String, String> extractStringsFromStructure(Object target, ArrayList<PathBuilder.PathPart> pathToPayload) {
private Map<String, String> extractStringsFromStructure(Object target, ArrayList<PathBuilder.PathPart> pathToPayload, int depth) {
HashMap<String, String> result = new HashMap<>();
Field[] fields = target.getClass().getDeclaredFields();
for (Field field : fields) {
Expand All @@ -106,7 +111,9 @@ private Map<String, String> extractStringsFromStructure(Object target, ArrayList
Object fieldValue = field.get(target);
ArrayList<PathBuilder.PathPart> newPathToPayload = new ArrayList<>(pathToPayload);
newPathToPayload.add(new PathBuilder.PathPart("object", field.getName()));
result.putAll(extractStringsRecursive(fieldValue, newPathToPayload));
result.putAll(extractStringsRecursive(fieldValue, newPathToPayload, depth + 1));
} catch (DangerousBodyException e) {
throw e;
} catch (IllegalAccessException | RuntimeException e) {
// pass-through
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;

import dev.aikido.agent_api.vulnerabilities.DangerousBodyException;

import java.util.Map;
import java.util.Objects;

public final class LooksLikeJWT {
private LooksLikeJWT() {}

public static final int MAX_JWT_PAYLOAD_BYTES = 8 * 1024;

public static Result tryDecodeAsJwt(String jwt) {
// Check if the JWT contains the required parts
if (jwt == null || !jwt.contains(".")) {
Expand All @@ -23,14 +27,24 @@ public static Result tryDecodeAsJwt(String jwt) {
return new Result(false, null);
}

byte[] decoded;
try {
// Decode the middle part (payload) of the JWT
String payload = new String(Base64.getUrlDecoder().decode(parts[1]));
decoded = Base64.getUrlDecoder().decode(parts[1]);
} catch (IllegalArgumentException ignored) {
return new Result(false, null);
}

if (decoded.length > MAX_JWT_PAYLOAD_BYTES) {
throw DangerousBodyException.jwtTooLarge();
}

String payload = new String(decoded);
try {
Gson gson = new Gson();
Map<String, Object> jwtPayload = gson.fromJson(payload, new TypeToken<Map<String, Object>>(){}.getType());

return new Result(true, jwtPayload);
} catch (StackOverflowError soe) {
throw DangerousBodyException.jwtTooLarge();
} catch (Exception ignored) {
return new Result(false, null);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package dev.aikido.agent_api.storage;

/**
* Thread-local flag recording whether the current request's remote IP is in the bypass list.
* Needed because bypassed requests intentionally do not set a context, but for Stored SSRF we still want to skip.
*/
public final class BypassedContextStore {
private BypassedContextStore() {}

private static final ThreadLocal<Boolean> store = ThreadLocal.withInitial(() -> false);

public static void setBypassed(boolean bypassed) {
store.set(bypassed);
}

public static boolean isBypassed() {
return store.get();
}

public static void clear() {
store.remove();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public BlockedResult isIpBlocked(String ip) {
// Check for allowed ip addresses (i.e. only one country is allowed to visit the site)
// Always allow access from private IP addresses (those include local IP addresses)
if (!isPrivateIp(ip) && !firewallLists.matchesAllowedIps(ip)) {
return new BlockedResult(true, "not in allowlist");
return new BlockedResult(true, "not allowed");
}

// Check for monitored IP addresses
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package dev.aikido.agent_api.vulnerabilities;

public class DangerousBodyException extends AikidoException {
public DangerousBodyException(String reason) {
super(generateDefaultMessage("Dangerous Body") + ": " + reason);
}

public static DangerousBodyException jwtTooLarge() {
return new DangerousBodyException("JWT payload too large");
}

public static DangerousBodyException bodyTooDeep() {
return new DangerousBodyException("Body is too deeply nested to scan");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public static void scanForGivenVulnerability(Vulnerabilities.Vulnerability vulne
break;
}
}
} catch (AikidoException ae) {
exception = Optional.of(ae);
} catch (Throwable e) {
logger.debug(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import dev.aikido.agent_api.background.Endpoint;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.storage.BypassedContextStore;
import dev.aikido.agent_api.storage.ServiceConfiguration;

import java.util.List;
Expand All @@ -12,6 +13,11 @@
public final class SkipVulnerabilityScanDecider {
private SkipVulnerabilityScanDecider() {}
public static boolean shouldSkipVulnerabilityScan(ContextObject context, boolean defaultIfNoContext) {
// Check if ip is bypassed, important still for stored ssrf where it runs without a context.
if (BypassedContextStore.isBypassed()) {
return true;
}

if (context == null) {
return defaultIfNoContext;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public BlockedOutboundException(String msg) {
}

public static BlockedOutboundException get() {
String defaultMsg = generateDefaultMessage("an outbound request");
String defaultMsg = generateDefaultMessage("an outbound connection");
return new BlockedOutboundException(defaultMsg);
}
}
Loading
Loading