Modify Rule S5883(Java): Education Framework (APPSEC-62) (#1147)

docs/allowed_framework_names.adoc

@@ -5,6 +5,7 @@
 * Entity Framework Core
 * Dapper
 // Java
+* Apache Commons
 * JSP
 * Servlet
 * Spring

rules/S2083/java/how-to-fix-it/java-se.adoc

@@ -9,13 +9,13 @@ h| Non-compliant code example
 |
 [source,java]
 ----
-@RestController
-public class ApiController
+@Controller
+public class ExampleController
 {
     static private String targetDirectory = "/path/to/target/directory/";
 
-    @GetMapping(value = "/endpoint")
-    public void endpoint(@RequestParam("filename") filename) throws IOException {
+    @GetMapping(value = "/delete")
+    public void delete(@RequestParam("filename") String filename) throws IOException {
 
         File file = new File(targetDirectory + filename);
         file.delete(); // Noncompliant
@@ -26,13 +26,13 @@ h| Compliant solution
 |
 [source,java]
 ----
-@RestController
-public class ApiController
+@Controller
+public class ExampleController
 {
     static private String targetDirectory = "/path/to/target/directory/";
 
-    @GetMapping(value = "/endpoint")
-    public void endpoint(@RequestParam("filename") filename) throws IOException {
+    @GetMapping(value = "/delete")
+    public void delete(@RequestParam("filename") String filename) throws IOException {
 
         File file = new File(targetDirectory + filename);
         String canonicalDestinationPath = file.getCanonicalPath();

rules/S5883/common/fix/code-rationale.adoc

@@ -0,0 +1,14 @@
+The following code uses the `find` command and expects the user to enter the
+name of a file to find on the system.
+
+It is vulnerable to arguments injection because untrusted data is inserted
+directly into the arguments of a process call without sanitization. +
+The application assumes that incoming data always consists of a specific range
+of characters and ignores that some characters might force the `find` command
+to start a shell.
+
+In this particular case, an attacker may remove files in `/some/folder` with the following string:
+
+----
+'*' -exec rm -rf {} \; 
+----

rules/S5883/java/how-to-fix-it/apache-commons.adoc

@@ -0,0 +1,46 @@
+=== How to fix it in Apache Commons
+
+include::../../common/fix/code-rationale.adoc[]
+
+[cols="a"]
+|===
+h| Non-compliant code example
+|
+[source,java]
+----
+@Controller
+public class ExampleController
+{
+    @GetMapping(value = "/find")
+    public void find(@RequestParam("filename") String filename) throws IOException {
+
+        CommandLine cmd = new CommandLine("/usr/bin/find . -iname " + filename); // Noncompliant
+    }
+}
+----
+h| Compliant solution
+|
+[source,java]
+----
+@Controller
+public class ExampleController
+{
+    @GetMapping(value = "/find")
+    public void find(@RequestParam("filename") String filename) throws IOException {
+
+        CommandLine cmd = new CommandLine("/usr/bin/find");
+        cmd.addArguments(new String[] {"/usr/bin/find", ".", "-iname", filename});
+    }
+}
+----
+|===
+
+=== How does this work?
+
+include::../../common/fix/introduction.adoc[]
+
+:sanitizationLib: org.apache.commons.exec.CommandLine.addArguments(String[] addArguments)
+include::../../common/fix/sanitize-meta-characters.adoc[]
+
+Here `org.apache.commons.exec.CommandLine.addArguments(String[] addArguments)` takes care of escaping the passed arguments and internally
+creates a single string given to the operating system to be executed.

rules/S5883/java/how-to-fix-it/java-se.adoc

@@ -0,0 +1,48 @@
+=== How to fix it in Java SE
+
+include::../../common/fix/code-rationale.adoc[]
+
+[cols="a"]
+|===
+h| Non-compliant code example
+|
+[source,java]
+----
+@Controller
+public class ExampleController
+{
+    @GetMapping(value = "/find")
+    public void find(@RequestParam("filename") String filename) throws IOException {
+
+        Runtime.getRuntime().exec("/usr/bin/find . -iname " + filename); // Noncompliant
+    }
+}
+----
+h| Compliant solution
+|
+[source,java]
+----
+@Controller
+public class ExampleController
+{
+    @GetMapping(value = "/find")
+    public void find(@RequestParam("filename") String filename) throws IOException {
+
+        String cmd1[] = new String[] {"/usr/bin/find", ".", "-iname", filename};
+        Process proc = Runtime.getRuntime().exec(cmd1); // Compliant
+    }
+}
+----
+|===
+
+++java.lang.Runtime++ is sometimes used over ++java.lang.ProcessBuilder++ due to ease of use. Flexibility in methods often introduces security issues as edge cases are easily missed. The compliant solution logic is also applied to ++java.lang.ProcessBuilder++.
+
+=== How does this work?
+
+include::../../common/fix/introduction.adoc[]
+
+:sanitizationLib: java.lang.Runtime.exec(String[] cmdarray)
+include::../../common/fix/sanitize-meta-characters.adoc[]
+
+Here `java.lang.Runtime.exec(String[] cmdarray)` takes care of escaping the passed arguments and internally
+creates a single string given to the operating system to be executed.