Skip to content

Count 404 requests with foreign extensions as attack wave scans #300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bitterpanda63 merged 1 commit into from
Jun 3, 2026
Merged

Count 404 requests with foreign extensions as attack wave scans #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion agent_api/src/main/java/dev/aikido/agent_api/collectors/WebResponseCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static void report(int statusCode) {
}

// Check for attack waves
if (AttackWaveDetectorStore.check(context)) {
if (AttackWaveDetectorStore.check(context, statusCode)) {
AttackQueue.add(
DetectedAttackWave.createAPIEvent(context)
);
Expand Down
7 changes: 4 additions & 3 deletions ...i/src/main/java/dev/aikido/agent_api/storage/attack_wave_detector/AttackWaveDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ public AttackWaveDetector(int attackWaveThreshold, long attackWaveTimeFrame,
/**
* Checks if the request is part of an attack wave.
*
* @param ctx the context object to check.
* @param ctx the context object to check.
* @param statusCode the HTTP response status code.
* @return true if an attack wave is detected and should be reported.
*/
public boolean check(ContextObject ctx) {
public boolean check(ContextObject ctx, int statusCode) {
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot Jun 1, 2026

Choose a reason for hiding this comment

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

Method 'check(ContextObject ctx, int statusCode)' still uses the ambiguous 'check' prefix; the name doesn't convey its side effects (mutating internal LRU caches and sending events) or returned semantic.

Details

✨ AI Reasoning
​The PR modified the AttackWaveDetector.check method signature to add a statusCode parameter. The method name still begins with 'check', which is the pattern the rule discourages. The change occurred at the method declaration line where the signature was updated. This keeps the same ambiguous 'check' prefix while altering the method's behavior/contract, which maintains and slightly worsens the clarity problem because callers must now pass an extra parameter to a method whose name doesn't indicate its side-effects or return semantics.

🔧 How do I fix it?
Replace 'check' with more descriptive verbs that indicate the function's action or purpose. Use 'validate' for validation, 'get' for retrieval, or 'is' for boolean checks. Ensure the name clearly communicates the function's intent and return type.

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

String ip = ctx.getRemoteAddress();
if (ip == null) {
return false;
Expand All @@ -59,7 +60,7 @@ public boolean check(ContextObject ctx) {
return false;
}

if (!isWebScanner(ctx)) {
if (!isWebScanner(ctx, statusCode)) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions .../main/java/dev/aikido/agent_api/storage/attack_wave_detector/AttackWaveDetectorStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public final class AttackWaveDetectorStore {
private AttackWaveDetectorStore() {
}

public static boolean check(ContextObject ctx) {
public static boolean check(ContextObject ctx, int statusCode) {
Copy link
Copy Markdown

@aikido-pr-checks aikido-pr-checks Bot Jun 1, 2026

Choose a reason for hiding this comment

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

Method 'check(ContextObject ctx, int statusCode)' uses the ambiguous 'check' prefix; its behavior (locking, forwarding to detector which mutates caches and may record events) isn't communicated by the name.

Details

✨ AI Reasoning
​The PR updated AttackWaveDetectorStore.check to accept a statusCode and forward it to the detector. The method name begins with 'check', which is discouraged: it doesn't indicate whether it performs queries, mutations, or reporting. The change touched the method signature and body, keeping the unclear name while changing its contract, thus worsening the clarity slightly by expanding the method's responsibilities without renaming.

🔧 How do I fix it?
Replace 'check' with more descriptive verbs that indicate the function's action or purpose. Use 'validate' for validation, 'get' for retrieval, or 'is' for boolean checks. Ensure the name clearly communicates the function's intent and return type.

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

mutex.lock();
try {
return detector.check(ctx);
return detector.check(ctx, statusCode);
} catch (Throwable e) {
logger.debug("An error occurred checking for attack waves: %s", e.getMessage());
return false;
Expand Down
14 changes: 13 additions & 1 deletion ...src/main/java/dev/aikido/agent_api/vulnerabilities/attack_wave_detection/PathChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,21 @@

import dev.aikido.agent_api.helpers.ArrayHelpers;

import java.util.Set;

import static dev.aikido.agent_api.helpers.ArrayHelpers.containsIgnoreCase;
import static dev.aikido.agent_api.vulnerabilities.attack_wave_detection.Paths.*;

public final class PathChecker {
private PathChecker() {
}

public static boolean isWebScanPath(String path) {
// Extensions that a Java app would not normally serve — only count as scan hits on 404
private static final Set<String> FOREIGN_EXTENSIONS = Set.of(
"php", "php3", "php4", "php5", "phtml"
);

public static boolean isWebScanPath(String path, int statusCode) {
String normalized = path.toLowerCase();
String[] segments = normalized.split("/");
String filename = ArrayHelpers.lastElement(segments);
Expand All @@ -25,6 +32,11 @@ public static boolean isWebScanPath(String path) {
if (containsIgnoreCase(FILE_EXTENSIONS, ext)) {
return true;
}
// Foreign extensions (e.g. PHP) are only suspicious when the response is 404.
// A 200 may mean the Java app is proxying to another backend.
if (FOREIGN_EXTENSIONS.contains(ext) && statusCode == 404) {
return true;
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions ...main/java/dev/aikido/agent_api/vulnerabilities/attack_wave_detection/WebScanDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
public final class WebScanDetector {
private WebScanDetector() {}

public static boolean isWebScanner(ContextObject ctx) {
public static boolean isWebScanner(ContextObject ctx, int statusCode) {
String method = ctx.getMethod();
if (method != null && isWebScanMethod(method)) {
return true;
}

String route = ctx.getRoute();
if (route != null && isWebScanPath(route)) {
if (route != null && isWebScanPath(route, statusCode)) {
return true;
}

Expand Down
16 changes: 8 additions & 8 deletions agent_api/src/test/java/attack_wave_detection/AttackWaveDetectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private static boolean checkDetector(AttackWaveDetector detector, String ip, boo
ctx = new EmptySampleContextObject("../etc/passwd", "/wp-config.php", "BADMETHOD");
}
ctx.setIp(ip);
return detector.check(ctx);
return detector.check(ctx, 404);
}

@Test
Expand Down Expand Up @@ -129,13 +129,13 @@ void testSlowWebScannerThirdInterval() throws InterruptedException {
@Test
void testItRespectsSamplesLimit() {
AttackWaveDetector detector = newAttackWaveDetector();
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "GET"));
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "GET"));
detector.check(new EmptySampleContextObject("", "/test2", "GET"));
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "POST"));
detector.check(new EmptySampleContextObject("", "/test3", "PUT"));
detector.check(new EmptySampleContextObject("", "/.env", "GET"));
detector.check(new EmptySampleContextObject("", "/test4", "BADMETHOD"));
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "GET"), 404);
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "GET"), 404);
detector.check(new EmptySampleContextObject("", "/test2", "GET"), 404);
detector.check(new EmptySampleContextObject("", "/../etc/passwd", "POST"), 404);
detector.check(new EmptySampleContextObject("", "/test3", "PUT"), 404);
detector.check(new EmptySampleContextObject("", "/.env", "GET"), 404);
detector.check(new EmptySampleContextObject("", "/test4", "BADMETHOD"), 404);
assertArrayEquals(
List.of(
new AttackWaveDetector.Sample("GET", "https://example.com/../etc/passwd"),
Expand Down
28 changes: 26 additions & 2 deletions agent_api/src/test/java/attack_wave_detection/PathCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ void testIsWebScanPath_WithDangerousPaths_ReturnsTrue() {
};
for (String path : dangerousPaths) {
assertTrue(
isWebScanPath(path),
isWebScanPath(path, 404),
"Expected '" + path + "' to be detected as a web scan path"
);
}
Expand All @@ -32,9 +32,33 @@ void testIsNotWebScanPath_WithSafePaths_ReturnsFalse() {
};
for (String path : safePaths) {
assertFalse(
isWebScanPath(path),
isWebScanPath(path, 404),
"Expected '" + path + "' to NOT be detected as a web scan path"
);
}
}

@Test
void testForeignExtensions_404_ReturnsTrue() {
assertTrue(isWebScanPath("/admin.php", 404),
"php extension with 404 should be a scan path");
assertTrue(isWebScanPath("/config.php5", 404),
"php5 extension with 404 should be a scan path");
assertTrue(isWebScanPath("/index.php3", 404),
"php3 extension with 404 should be a scan path");
assertTrue(isWebScanPath("/index.php4", 404),
"php4 extension with 404 should be a scan path");
assertTrue(isWebScanPath("/page.phtml", 404),
"phtml extension with 404 should be a scan path");
}

@Test
void testForeignExtensions_200_ReturnsFalse() {
assertFalse(isWebScanPath("/admin.php", 200),
"php extension with 200 should NOT be a scan path (app may proxy to PHP backend)");
assertFalse(isWebScanPath("/config.php5", 200),
"php5 extension with 200 should NOT be a scan path");
assertFalse(isWebScanPath("/page.phtml", 200),
"phtml extension with 200 should NOT be a scan path");
}
}
6 changes: 3 additions & 3 deletions agent_api/src/test/java/attack_wave_detection/WebScanDetectorBenchmarkTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ void testPerformance() {
int iterations = 25_000;
long start = System.nanoTime();
for (int i = 0; i < iterations; i++) {
WebScanDetector.isWebScanner(getTestContext("/wp-config.php", "GET", "1"));
WebScanDetector.isWebScanner(getTestContext("/vulnerable", "GET", "1'; DROP TABLE users; --"));
WebScanDetector.isWebScanner(getTestContext("/", "GET", "1"));
WebScanDetector.isWebScanner(getTestContext("/wp-config.php", "GET", "1"), 404);
WebScanDetector.isWebScanner(getTestContext("/vulnerable", "GET", "1'; DROP TABLE users; --"), 200);
WebScanDetector.isWebScanner(getTestContext("/", "GET", "1"), 200);
}
long end = System.nanoTime();
double timePerCheck = (double) (end - start) / iterations / 3 / 1_000_000; // Convert nanoseconds to milliseconds
Expand Down
40 changes: 20 additions & 20 deletions agent_api/src/test/java/attack_wave_detection/WebScanDetectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,48 +20,48 @@ private static ContextObject createTestContext(String path, String method, Map<S
@Test
public void testIsWebScanner_PositiveCases() {
// Test suspicious paths
assertTrue(WebScanDetector.isWebScanner(createTestContext("/wp-config.php", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.env", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/test/.env.bak", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.git/config", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.aws/config", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/../secret", "GET", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/wp-config.php", "GET", Map.of()), 404));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.env", "GET", Map.of()), 404));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/test/.env.bak", "GET", Map.of()), 404));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.git/config", "GET", Map.of()), 404));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/.aws/config", "GET", Map.of()), 404));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/../secret", "GET", Map.of()), 404));

// Test suspicious method
assertTrue(WebScanDetector.isWebScanner(createTestContext("/", "BADMETHOD", Map.of())));
assertTrue(WebScanDetector.isWebScanner(createTestContext("/", "BADMETHOD", Map.of()), 200));

// Test suspicious query parameters
assertTrue(WebScanDetector.isWebScanner(
createTestContext("/", "GET", Map.of("test", List.of("SELECT * FROM admin")))
createTestContext("/", "GET", Map.of("test", List.of("SELECT * FROM admin"))), 200
));
assertTrue(WebScanDetector.isWebScanner(
createTestContext("/", "GET", Map.of("test", List.of("../etc/passwd")))
createTestContext("/", "GET", Map.of("test", List.of("../etc/passwd"))), 200
));
}

@Test
public void testIsWebScanner_NegativeCases() {
// Test safe paths
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", "POST", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/api/v1/users", "GET", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/public/index.html", "GET", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/static/js/app.js", "GET", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/uploads/image.png", "GET", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", "POST", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/api/v1/users", "GET", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/public/index.html", "GET", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/static/js/app.js", "GET", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext("/uploads/image.png", "GET", Map.of()), 200));

// Test safe query parameters
assertFalse(WebScanDetector.isWebScanner(
createTestContext("/", "GET", Map.of("test", List.of("1'")))
createTestContext("/", "GET", Map.of("test", List.of("1'"))), 200
));
assertFalse(WebScanDetector.isWebScanner(
createTestContext("/", "GET", Map.of("test", List.of("abcd")))
createTestContext("/", "GET", Map.of("test", List.of("abcd"))), 200
));
}

@Test
public void testWithNull() {
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", "POST", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext(null, "POST", Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", null, Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext(null, null, Map.of())));
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", "POST", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext(null, "POST", Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext("graphql", null, Map.of()), 200));
assertFalse(WebScanDetector.isWebScanner(createTestContext(null, null, Map.of()), 200));
}
}
Loading