Skip to content

re-use vulnerability type/detector instances to avoid unnecessary mem usage #247

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

re-use vulnerability type/detector instances to avoid unnecessary mem usage #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a6219c2
Create only 1 instance for each of the vulnerability types
bitterpanda63 Nov 17, 2025
283582f
Update code to use this vulnerabiliy instance
bitterpanda63 Nov 17, 2025
10a3e96
Make the SSRFDetector static
bitterpanda63 Nov 21, 2025
bedc44b
Create INSTANCEs for the remaining detectors
bitterpanda63 Nov 21, 2025
8a70bb6
Update PathTraversalDetectorTest cases to use this new INSTANCE
bitterpanda63 Nov 21, 2025
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 agent_api/src/main/java/dev/aikido/agent_api/collectors/CommandCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public static void report(Object command) {
StatisticsStore.registerCall("runtime.Exec", OperationKind.EXEC_OP);

// scan
Vulnerabilities.Vulnerability vulnerability = new Vulnerabilities.ShellInjectionVulnerability();
Scanner.scanForGivenVulnerability(vulnerability, "runtime.Exec", new String[]{commandStr});
Scanner.scanForGivenVulnerability(Vulnerabilities.SHELL_INJECTION, "runtime.Exec", new String[]{commandStr});
}
}
}
2 changes: 1 addition & 1 deletion agent_api/src/main/java/dev/aikido/agent_api/collectors/DNSRecordCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
}
logger.debug("Hostname: %s, Port: %s, IPs: %s", hostnameEntry.getHostname(), hostnameEntry.getPort(), ipAddresses);

Attack attack = new SSRFDetector().run(
Attack attack = SSRFDetector.run(
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Replacing instance call with static 'SSRFDetector.run' may introduce shared mutable state access and race conditions if run uses non-thread-safe state.

Details

✨ AI Reasoning
​​1) The change replaced an instance method call 'new SSRFDetector().run(...)' with a static call 'SSRFDetector.run(...)' at the modified line; 2) Calling a static method can introduce shared mutable state access or make previously thread-confined behavior global, increasing risk of race conditions if SSRFDetector.run uses mutable/static fields; 3) This harms maintainability and thread-safety because it changes object lifecycle and sharing semantics in a multi-threaded environment; 4) The violation is true because the diff introduced the change from instance to static invocation which can create thread-safety concerns not present before; 5) It is plausible to fix within the PR by ensuring run is thread-safe or reverting to instance usage.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

hostname, hostnameEntry.getPort(), ipAddresses, OPERATION_NAME
);
if (attack == null) {
Expand Down
7 changes: 3 additions & 4 deletions agent_api/src/main/java/dev/aikido/agent_api/collectors/FileCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,17 @@ public static void report(Object filePath, String operation, int depth) {
logger.trace("Scan on %s for file: %s", operation, filePath);
StatisticsStore.registerCall(operation, OperationKind.FS_OP);

Vulnerabilities.Vulnerability vulnerability = new Vulnerabilities.PathTraversalVulnerability();
if (filePath instanceof String filePathString) {
Scanner.scanForGivenVulnerability(vulnerability, operation, new String[]{filePathString});
Scanner.scanForGivenVulnerability(Vulnerabilities.PATH_TRAVERSAL, operation, new String[]{filePathString});
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Reusing shared Vulnerabilities.PATH_TRAVERSAL may introduce data races if Vulnerability instances are mutable or used concurrently.

Details

✨ AI Reasoning
​​1) The change replaces per-call creation of a Vulnerabilities.Vulnerability instance with a shared reference Vulnerabilities.PATH_TRAVERSAL and uses it in Scanner.scanForGivenVulnerability.
2) If Vulnerabilities.PATH_TRAVERSAL or the Vulnerability instances are mutable or Scanner modifies the vulnerability object, reusing a single static instance can introduce race conditions when accessed concurrently by multiple threads.
3) This harms maintainability because it changes object lifetimes and introduces potential thread-safety problems that did not exist when each call created its own instance.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

} else if (filePath instanceof URI filePathURI) {
// File(...) Constructor also accepts URI objects, but path remains the same
// So we just extract the path here : (i.e. new File("file:///../test.txt") --> "/../test.txt")
String filePathString = filePathURI.getPath();
Scanner.scanForGivenVulnerability(vulnerability, operation, new String[]{filePathString});
Scanner.scanForGivenVulnerability(Vulnerabilities.PATH_TRAVERSAL, operation, new String[]{filePathString});
} else if (filePath instanceof Path filePathAsPath) {
// Some functions on Path also accept other paths
String filePathString = filePathAsPath.toString();
Scanner.scanForGivenVulnerability(vulnerability, operation, new String[]{filePathString});
Scanner.scanForGivenVulnerability(Vulnerabilities.PATH_TRAVERSAL, operation, new String[]{filePathString});
} else if (filePath instanceof Object[] filePaths) {
// In Paths.get() sometimes you can have multiple paths provided, scan them individually :
if (depth >= MAX_RECURSION_DEPTH) {
Expand Down
3 changes: 1 addition & 2 deletions agent_api/src/main/java/dev/aikido/agent_api/collectors/SQLCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ public static void report(String sql, String dialect, String operation) {
StatisticsStore.registerCall(operation, OperationKind.SQL_OP);

// scan
Vulnerabilities.Vulnerability vulnerability = new Vulnerabilities.SQLInjectionVulnerability();
Scanner.scanForGivenVulnerability(vulnerability, operation, new String[]{sql, dialect});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, operation, new String[]{sql, dialect});
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Reusing a shared Vulnerabilities.SQL_INJECTION instance may introduce thread-safety issues if the vulnerability object is mutable or mutated during scanning.

Details

✨ AI Reasoning
​​1) The code replaces creating a new Vulnerabilities.SQLInjectionVulnerability() per call with reusing a shared instance Vulnerabilities.SQL_INJECTION when calling Scanner.scanForGivenVulnerability.
​2) Reusing a shared/static vulnerability instance can introduce data races or inconsistent behavior if that instance is mutable or if Scanner or other code mutates it during scanning, harming thread-safety in a multi-threaded environment.
​3) This is a change introduced by the diff (previously per-call instantiation avoided shared mutable state), so the risk is newly introduced by this PR.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

}
}
27 changes: 18 additions & 9 deletions agent_api/src/main/java/dev/aikido/agent_api/vulnerabilities/Vulnerabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,56 +3,65 @@
import dev.aikido.agent_api.vulnerabilities.path_traversal.PathTraversalDetector;
import dev.aikido.agent_api.vulnerabilities.shell_injection.ShellInjectionDetector;
import dev.aikido.agent_api.vulnerabilities.sql_injection.SqlDetector;
import dev.aikido.agent_api.vulnerabilities.ssrf.SSRFDetector;

public final class Vulnerabilities {
private Vulnerabilities() {}
public interface Vulnerability {
Detector getDetector();
String getKind();
}
public static final class SQLInjectionVulnerability implements Vulnerability {

private static final class SQLInjectionVulnerability implements Vulnerability {
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Switched from creating new Detector instances to using shared INSTANCE singletons, which may expose non-thread-safe Detector state to concurrent threads.

Details

✨ AI Reasoning
​​1) The changes replaced per-call creation of Detector objects with shared singletons (e.g., SqlDetector.INSTANCE, PathTraversalDetector.INSTANCE, ShellInjectionDetector.INSTANCE) and added public static final Vulnerability instances; 2) Sharing mutable or non-thread-safe Detector instances across threads can introduce race conditions or incorrect behavior if those Detector implementations are not designed for concurrent use; 3) This harms maintainability and safety because callers that previously got fresh Detector instances now share state implicitly; 4) Fixing would meaningfully improve safety by ensuring Detector implementations are thread-safe or preserving per-call instantiation; 5) Using singletons for objects that may have internal mutable state is a common source of concurrency bugs; 6) This is a targeted change and can be addressed in this PR by reverting to per-call instantiation or documenting/making Detectors thread-safe.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

@Override
public Detector getDetector() {
return new SqlDetector();
return SqlDetector.INSTANCE;
}
@Override
public String getKind() {
return "sql_injection";
}
}
public static final class PathTraversalVulnerability implements Vulnerability {
public static final Vulnerability SQL_INJECTION = new SQLInjectionVulnerability();

private static final class PathTraversalVulnerability implements Vulnerability {
@Override
public Detector getDetector() {
return new PathTraversalDetector();
return PathTraversalDetector.INSTANCE;
}
@Override
public String getKind() {
return "path_traversal";
}
}
public static final class SSRFVulnerability implements Vulnerability {
public static final Vulnerability PATH_TRAVERSAL = new PathTraversalVulnerability();

private static final class SSRFVulnerability implements Vulnerability {
@Override
public Detector getDetector() { return null; }
@Override
public String getKind() {
return "ssrf";
}
}
public static final class StoredSSRFVulnerability implements Vulnerability {
public static final Vulnerability SSRF = new SSRFVulnerability();

private static final class StoredSSRFVulnerability implements Vulnerability {
@Override
public Detector getDetector() { return null; }
@Override
public String getKind() {
return "stored_ssrf";
}
}
public static final class ShellInjectionVulnerability implements Vulnerability {
public static final Vulnerability STORED_SSRF = new StoredSSRFVulnerability();

private static final class ShellInjectionVulnerability implements Vulnerability {
@Override
public Detector getDetector() { return new ShellInjectionDetector(); }
public Detector getDetector() { return ShellInjectionDetector.INSTANCE; }
@Override
public String getKind() {
return "shell_injection";
}
}
public static final Vulnerability SHELL_INJECTION = new ShellInjectionVulnerability();
}
2 changes: 2 additions & 0 deletions .../main/java/dev/aikido/agent_api/vulnerabilities/path_traversal/PathTraversalDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import static dev.aikido.agent_api.vulnerabilities.path_traversal.UnsafePathPartsChecker.containsUnsafePathParts;

public class PathTraversalDetector implements Detector {
public static final PathTraversalDetector INSTANCE = new PathTraversalDetector();

/**
* @param userInput, this is the user input which we will evaluate
* @param arguments, Includes one element : filename
Expand Down
2 changes: 2 additions & 0 deletions ...ain/java/dev/aikido/agent_api/vulnerabilities/shell_injection/ShellInjectionDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import static dev.aikido.agent_api.vulnerabilities.shell_injection.ShellSyntaxChecker.containsShellSyntax;

public class ShellInjectionDetector implements Detector {
public static final ShellInjectionDetector INSTANCE = new ShellInjectionDetector();

@Override
public DetectorResult run(String userInput, String[] arguments) {
if (userInput.isEmpty() || arguments == null || arguments.length == 0 || arguments[0] == null) {
Expand Down
2 changes: 2 additions & 0 deletions agent_api/src/main/java/dev/aikido/agent_api/vulnerabilities/sql_injection/SqlDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import java.util.regex.Pattern;

public class SqlDetector implements Detector {
public static final SqlDetector INSTANCE = new SqlDetector();
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 21, 2025

Choose a reason for hiding this comment

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

Declaring INSTANCE before the static logger risks static initialization-order problems if the instance uses the logger during construction.

Details

✨ AI Reasoning
​​1) The change adds a public static final INSTANCE at line 11 which instantiates SqlDetector during class static initialization.
​2) This can harm maintainability/runtime safety because static fields are initialized in textual order; INSTANCE is created before the static logger field (line 12), so if SqlDetector's constructor or any instance initialization uses the logger, it would see a null logger and could throw or behave incorrectly.
​3) This is a real initialization-order hazard introduced by this diff because the logger field was present before but the new INSTANCE was added earlier in the file; the issue did not exist prior to inserting the INSTANCE before the logger. Therefore this is a valid problem introduced by the change.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

private static final Logger logger = LogManager.getLogger(SqlDetector.class);

/**
* @param userInput contains the user input which we want to scan
* @param arguments contains: [query, dialect]
Expand Down
10 changes: 3 additions & 7 deletions agent_api/src/main/java/dev/aikido/agent_api/vulnerabilities/ssrf/SSRFDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,18 @@
import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.vulnerabilities.Attack;
import dev.aikido.agent_api.vulnerabilities.Detector;
import dev.aikido.agent_api.vulnerabilities.Vulnerabilities;

import java.util.HashSet;
import java.util.List;
import java.util.Map;

import static dev.aikido.agent_api.helpers.ShouldBlockHelper.shouldBlock;
import static dev.aikido.agent_api.helpers.StackTrace.getCurrentStackTrace;
import static dev.aikido.agent_api.vulnerabilities.ssrf.FindHostnameInContext.findHostnameInContext;
import static dev.aikido.agent_api.vulnerabilities.ssrf.IsPrivateIP.containsPrivateIP;
import static dev.aikido.agent_api.vulnerabilities.ssrf.PrivateIPRedirectFinder.isRedirectToPrivateIP;
import static dev.aikido.agent_api.vulnerabilities.ssrf.imds.Resolver.resolvesToImdsIp;

public class SSRFDetector {
public Attack run(String hostname, int port, List<String> ipAddresses, String operation) {
public final class SSRFDetector {
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Changing SSRFDetector to final and making run(...) static breaks the previous public API (instance method -> static).

Details

🔧 How do I fix it?
Support both old and new parameter names during transition periods. Add new optional parameters with defaults. Keep existing response fields while adding new ones. Focus on backwards compatibility.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

public static Attack run(String hostname, int port, List<String> ipAddresses, String operation) {
if(hostname == null || hostname.isEmpty()) {
return null;
}
Expand All @@ -39,7 +35,7 @@ public Attack run(String hostname, int port, List<String> ipAddresses, String op
if(attackFindings != null) {
return new Attack(
operation,
new Vulnerabilities.SSRFVulnerability(),
Vulnerabilities.SSRF,
attackFindings.source(),
attackFindings.pathToPayload(),
/*metadata*/ Map.of(
Expand Down
2 changes: 1 addition & 1 deletion agent_api/src/main/java/dev/aikido/agent_api/vulnerabilities/ssrf/StoredSSRFDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public Attack run(String hostname, List<String> ipAddresses, String operation) {

return new Attack(
operation,
new Vulnerabilities.StoredSSRFVulnerability(),
Vulnerabilities.STORED_SSRF,
Copy link

@aikido-pr-checks aikido-pr-checks bot Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Replacing a per-call Vulnerability object with a shared Vulnerabilities.STORED_SSRF instance may introduce unsafe shared mutable state access across threads.

Details

✨ AI Reasoning
​​1) The method creates an Attack object and previously instantiated a fresh Vulnerabilities.StoredSSRFVulnerability per call; the change replaces that with a shared Vulnerabilities.STORED_SSRF instance.
​2) Reusing a shared vulnerability instance can introduce shared mutable state if the Vulnerability object is not immutable, risking data races or accidental cross-request mutation in multi-threaded environments.
​3) This harms maintainability and could cause concurrency bugs if the shared instance is mutated elsewhere.
​4) Changing from per-call allocation to a shared instance is a localized change and its thread-safety implications are directly introduced by this PR.
​5) It's not known here whether the instance is immutable, so the conservative action is to flag the potential issue.
​6) The fix may be to ensure the shared object is immutable or to synchronize access, which is reasonable within the PR scope.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

null, // source is null for stored attacks
"", // path is empty
Map.of(
Expand Down
10 changes: 4 additions & 6 deletions agent_api/src/test/java/vulnerabilities/AttackTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ public class AttackTest {
public void testAttackConstructor() {
// Arrange
String operation = "SQL Injection";
Vulnerabilities.Vulnerability vulnerability = new Vulnerabilities.SQLInjectionVulnerability();
String source = "User Input";
String pathToPayload = "/api/vulnerable";
Map<String, String> metadata = new HashMap<>();
Expand All @@ -26,11 +25,11 @@ public void testAttackConstructor() {
User user = new User("id", "name", "1.1.1.1", 0);

// Act
Attack attack = new Attack(operation, vulnerability, source, pathToPayload, metadata, payload, stack, user);
Attack attack = new Attack(operation, Vulnerabilities.SQL_INJECTION, source, pathToPayload, metadata, payload, stack, user);

// Assert
assertEquals(operation, attack.operation);
assertEquals(vulnerability.getKind(), attack.kind);
assertEquals("sql_injection", attack.kind);
assertEquals(source, attack.source);
assertEquals(pathToPayload, attack.pathToPayload);
assertEquals(metadata, attack.metadata);
Expand All @@ -47,7 +46,6 @@ public void testAttackConstructor() {
public void testAttackWithEmptyMetadata() {
// Arrange
String operation = "XSS Attack";
Vulnerabilities.Vulnerability vulnerability = new Vulnerabilities.SQLInjectionVulnerability();
String source = "User Input";
String pathToPayload = "/api/vulnerable";
Map<String, String> metadata = new HashMap<>(); // Empty metadata
Expand All @@ -56,11 +54,11 @@ public void testAttackWithEmptyMetadata() {
User user = new User("123", "name", "1.1.1.1", 0);

// Act
Attack attack = new Attack(operation, vulnerability, source, pathToPayload, metadata, payload, stack, user);
Attack attack = new Attack(operation, Vulnerabilities.SQL_INJECTION, source, pathToPayload, metadata, payload, stack, user);

// Assert
assertEquals(operation, attack.operation);
assertEquals(vulnerability.getKind(), attack.kind);
assertEquals("sql_injection", attack.kind);
assertEquals(source, attack.source);
assertEquals(pathToPayload, attack.pathToPayload);
assertEquals(metadata, attack.metadata);
Expand Down
30 changes: 15 additions & 15 deletions agent_api/src/test/java/vulnerabilities/ScannerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ void testScanForGivenVulnerability_ContextIsNull() {
void testScanSafeSQLCode() throws InterruptedException {
ServiceConfigStore.updateBlocking(true);
// Safe :
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT", "postgresql"});
// Argument-mismatch, safe :
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM"});
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "1", "2", "3"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "1", "2", "3"});

// Unsafe :
AttackQueue.clear();
assertThrows(SQLInjectionException.class, () -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
DetectedAttack.DetectedAttackEvent attackEvent = (DetectedAttack.DetectedAttackEvent) AttackQueue.get();
assertEquals("SELECT * FRO", attackEvent.attack().payload());
Expand All @@ -98,14 +98,14 @@ void testScanSafeSQLCode() throws InterruptedException {
void testScanSafeSQLCodeButBlockingFalse() {
ServiceConfigStore.updateBlocking(false);
// Safe :
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT", "postgresql"});
// Argument-mismatch, safe :
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM"});
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "1", "2", "3"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "1", "2", "3"});

// Unsafe :
assertDoesNotThrow(() -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
}

Expand All @@ -114,22 +114,22 @@ void testForceProtectionOff() {
ServiceConfigStore.updateBlocking(true);
// Thread cache does not force any protection off :
assertThrows(SQLInjectionException.class, () -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
// Set to protection forced off route :
Context.set(new SampleContextObject3("/api2/test/2/4"));
assertDoesNotThrow(() -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
Context.set(new SampleContextObject3("/api2/"));
assertDoesNotThrow(() -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});

// Set to IP where route exists but protection is not forced off :
Context.set(new SampleContextObject3("/api3/test"));
assertThrows(SQLInjectionException.class, () -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
}

Expand All @@ -138,7 +138,7 @@ void testDoesNotRunWithContextNull() {
ServiceConfigStore.updateBlocking(true);
Context.set(null);
assertDoesNotThrow(() -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
}

Expand All @@ -148,15 +148,15 @@ void TestStillThrowsWithConfigStoreEmptyButBlockingEnabled() {
ServiceConfigStore.updateFromAPIResponse(emptyAPIResponse);
ServiceConfigStore.updateBlocking(true);
assertThrows(SQLInjectionException.class, () -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
}

@Test
void TestDoesNotThrowWithEmptyAPIResponse() {
ServiceConfigStore.updateFromAPIResponse(emptyAPIResponse);
assertDoesNotThrow(() -> {
Scanner.scanForGivenVulnerability(new Vulnerabilities.SQLInjectionVulnerability(), "operation", new String[]{"SELECT * FROM", "postgresql"});
Scanner.scanForGivenVulnerability(Vulnerabilities.SQL_INJECTION, "operation", new String[]{"SELECT * FROM", "postgresql"});
});
}
}
Loading
Loading