Fix setSafe() side effect bug.

OWASP-Benchmark · Feb 20, 2024 · 4ca0d58 · 4ca0d58
1 parent 42770df
commit 4ca0d58
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 8 deletions.
diff --git a/library/src/main/java/org/owasp/benchmarkutils/entities/CliArgExecutableTestCaseInput.java b/library/src/main/java/org/owasp/benchmarkutils/entities/CliArgExecutableTestCaseInput.java
@@ -33,7 +33,9 @@ public List<RequestVariable> getArgs() {
     }
 
     public void setArgs(List<RequestVariable> args) {
-        this.args = args;
+        // Copy the given list so setSafe() does not affect other CliArgExecutableTestCaseInput
+        // objects.
+        this.args = new ArrayList<>(args);
     }
 
     public void addArg(RequestVariable arg) {
@@ -48,14 +50,23 @@ public CliRequest buildAttackRequest() {
         //		// FIXME: This will break if the command string has arguments that contain spaces.
         //		executeArgs.addAll(Arrays.asList(getCommand().split(" ")));
         //		executeArgs.addAll(getArgs());
-
-        setSafe(false);
-        return new CliRequest(getCommand(), getArgs());
+        ArrayList<RequestVariable> argsCopy = new ArrayList<>();
+        for (RequestVariable arg : args) {
+            RequestVariable argCopy = new RequestVariable(arg);
+            argCopy.setSafe(false);
+            argsCopy.add(argCopy);
+        }
+        return new CliRequest(getCommand(), argsCopy);
     }
 
     public CliRequest buildSafeRequest() {
-        setSafe(true);
-        return new CliRequest(getCommand(), getArgs());
+        ArrayList<RequestVariable> argsCopy = new ArrayList<>();
+        for (RequestVariable arg : args) {
+            RequestVariable argCopy = new RequestVariable(arg);
+            argCopy.setSafe(true);
+            argsCopy.add(argCopy);
+        }
+        return new CliRequest(getCommand(), argsCopy);
     }
 
     public void setSafe(boolean isSafe) {

diff --git a/library/src/main/java/org/owasp/benchmarkutils/entities/CliRequest.java b/library/src/main/java/org/owasp/benchmarkutils/entities/CliRequest.java
@@ -12,12 +12,14 @@ public class CliRequest {
     public CliRequest(String command, List<RequestVariable> args) {
         super();
         this.command = command;
-        this.args = args;
+        this.args = new ArrayList<RequestVariable>(args);
     }
 
     public CliRequest(String command, RequestVariable arg) {
         super();
         this.command = command;
+        // Make a copy of the given args list so that when setSafe() changes elements, the changes
+        // do not affect other CliRequest objects.
         this.args = new ArrayList<RequestVariable>(Arrays.asList(arg));
     }
 
@@ -34,12 +36,20 @@ public List<RequestVariable> getArgs() {
     }
 
     public void setArgs(List<RequestVariable> args) {
-        this.args = args;
+        this.args = new ArrayList<RequestVariable>(args);
     }
 
     //	public List<String> getExecuteArgs() {
     //    	List<String> executeArgs = Arrays.asList(getCommand().split(" "));
     //    	executeArgs.addAll(getArgs());
     //    	return executeArgs;
     //	}
+
+    public String toString() {
+        ArrayList<String> executeArgs = new ArrayList<>(Arrays.asList(command.split(" ")));
+        for (RequestVariable arg : args) {
+            executeArgs.add(arg.getValue());
+        }
+        return String.join(" ", executeArgs);
+    }
 }
diff --git a/library/src/main/java/org/owasp/benchmarkutils/entities/RequestVariable.java b/library/src/main/java/org/owasp/benchmarkutils/entities/RequestVariable.java
@@ -45,6 +45,19 @@ public RequestVariable(
         isSafe = name.equals(safeName) && value.equals(safeValue);
     }
 
+    public RequestVariable(RequestVariable otherRequestVariable) {
+        super();
+        this.name = otherRequestVariable.getName();
+        this.value = otherRequestVariable.getValue();
+        this.attackName = otherRequestVariable.getAttackName();
+        this.attackValue = otherRequestVariable.getAttackValue();
+        this.safeName = otherRequestVariable.getSafeName();
+        this.safeValue = otherRequestVariable.getSafeValue();
+        if (name == null) throw new NullPointerException("name parameter cannot be null");
+        if (value == null) throw new NullPointerException("value parameter cannot be null");
+        isSafe = name.equals(safeName) && value.equals(safeValue);
+    }
+
     @XmlAttribute
     @NotNull
     public String getName() {

diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/tools/BenchmarkCrawlerVerification.java b/plugin/src/main/java/org/owasp/benchmarkutils/tools/BenchmarkCrawlerVerification.java
@@ -184,6 +184,7 @@ protected void crawl(TestSuite testSuite) throws Exception {
                         safeExecutor = new CliExecutor(safeRequest);
 
                         // Send the next test case request with its attack payload
+                        System.out.println("Executing attack request: " + attackRequest);
                         attackPayloadResponseInfo = execute(attackRequest);
                         ////		                executeArgs.add(payload);
                         //		                ProcessBuilder builder = new
@@ -201,6 +202,7 @@ protected void crawl(TestSuite testSuite) throws Exception {
                         safePayloadResponseInfo = null;
                         if (!testCase.isUnverifiable()) {
                             // Send the next test case request with its safe payload
+                            System.out.println("Executing safe request: " + safeRequest);
                             safePayloadResponseInfo = execute(safeRequest);
                             responseInfoList.add(safePayloadResponseInfo);