CWE-78

Bryant001 · Feb 2, 2021 · 8f87be9 · 8f87be9
1 parent eb1154c
commit 8f87be9
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 36 deletions.
diff --git a/app/src/main/java/com/veracode/verademo/controller/ToolsController.java b/app/src/main/java/com/veracode/verademo/controller/ToolsController.java
@@ -3,6 +3,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.util.concurrent.TimeUnit;
 
 import javax.servlet.ServletContext;
 
@@ -39,7 +40,7 @@ public String tools(
 		model.addAttribute("ping", host != null ? ping(host) : "");
 
 		if (fortuneFile == null) {
-			fortuneFile = "funny.txt";
+			fortuneFile = "literature";
 		}
 		model.addAttribute("fortunes", fortune(fortuneFile));
 
@@ -52,9 +53,10 @@ private String ping(String host)
 		Process proc;
 		try {
 			/* START BAD CODE */
-			proc = Runtime.getRuntime().exec("ping -c1 " + host);
+			proc = Runtime.getRuntime().exec(new String[]{"bash", "-c", "ping -c1 " + host});
 			/* END BAD CODE */
 
+			proc.waitFor(5, TimeUnit.SECONDS);
 			InputStreamReader isr = new InputStreamReader(proc.getInputStream());
 			BufferedReader br = new BufferedReader(isr);
 
@@ -63,10 +65,18 @@ private String ping(String host)
 			while ((line = br.readLine()) != null) {
 				output += line + "\n";
 			}
+
+
+
+			logger.info(proc.exitValue());
 		}
 		catch (IOException ex) {
 			logger.error(ex);
 		}
+		catch (InterruptedException ex) {
+			logger.error(ex);
+		}
+
 		return output;
 	}
 
@@ -78,9 +88,10 @@ private String fortune(String fortuneFile)
 		Process proc;
 		try {
 			/* START BAD CODE */
-			proc = Runtime.getRuntime().exec(cmd);
+			proc = Runtime.getRuntime().exec(new String[] {"bash", "-c", cmd});
 			/* END BAD CODE */
 
+			proc.waitFor(5, TimeUnit.SECONDS);
 			InputStreamReader isr = new InputStreamReader(proc.getInputStream());
 			BufferedReader br = new BufferedReader(isr);
 
@@ -93,6 +104,10 @@ private String fortune(String fortuneFile)
 		catch (IOException ex) {
 			logger.error(ex);
 		}
+		catch (InterruptedException ex) {
+			logger.error(ex);
+		}
+
 		return output;
 	}
 }
diff --git a/app/src/main/webapp/WEB-INF/views/login.jsp b/app/src/main/webapp/WEB-INF/views/login.jsp
@@ -34,7 +34,7 @@
 		<div class="header clearfix">
 			<nav>
 				<ul class="nav nav-pills pull-right">
-					<li role="presentation"><a href="reset">Reset</a></li>
+					<!-- <li role="presentation"><a href="reset">Reset</a></li> -->
 					<li role="presentation"><a href="login" class="active">Login</a></li>
 					<li role="presentation"><a href="register">Register</a></li>
 				</ul>

diff --git a/app/src/main/webapp/WEB-INF/views/register-finish.jsp b/app/src/main/webapp/WEB-INF/views/register-finish.jsp
@@ -34,7 +34,7 @@
 		<div class="header clearfix">
 			<nav>
 				<ul class="nav nav-pills pull-right">
-					<li role="presentation"><a href="reset">Reset</a></li>
+					<!-- <li role="presentation"><a href="reset">Reset</a></li> -->
 					<li role="presentation"><a href="login">Login</a></li>
 					<li role="presentation"><a href="register" class="active">Register</a></li>
 				</ul>

diff --git a/app/src/main/webapp/WEB-INF/views/register.jsp b/app/src/main/webapp/WEB-INF/views/register.jsp
@@ -34,7 +34,7 @@
 		<div class="header clearfix">
 			<nav>
 				<ul class="nav nav-pills pull-right">
-					<li role="presentation"><a href="reset">Reset</a></li>
+					<!-- <li role="presentation"><a href="reset">Reset</a></li> -->
 					<li role="presentation"><a href="login">Login</a></li>
 					<li role="presentation"><a href="register" class="active">Register</a></li>
 				</ul>

diff --git a/docs/flaws/cwe-77-command-injection.md b/docs/flaws/cwe-77-command-injection.md
diff --git a/docs/flaws/cwe-78-command-injection.md b/docs/flaws/cwe-78-command-injection.md
@@ -0,0 +1,32 @@
+CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') in VeraDemo
+==============================================================================================================
+
+VeraDemo has a page called **Tools**, available at `/verademo/tools`.
+
+This has functionality to let you check the uptime of a host or show a fortune.
+Unfortunately they do so by directly executing shell commands without any validation.
+
+Exploit
+-------
+We can exploit this like so:
+1. Go to `/verademo/tools`.
+2. For Host enter: `127.0.0.1 ; cat /etc/passwd`.
+3. Click `Check`.
+4. Observe the file listing as evidence that OS command injection has occurred.
+5. Right click on the dropdown with "literature" and click Inspect to open the browser's developer tools.
+6. Change the first option value from `literature` to `; ls -al /`.
+7. Press the Change button in the browser.
+8. Observe the directory listing as evidence that OS command injection has occurred.
+
+Mitigate
+--------
+Validate host and fortunes definition.
+
+Remediate
+---------
+Validate the host and use a whitelist for allowed fortunes files.
+
+Resources
+---------
+* [CWE 78](https://cwe.mitre.org/data/definitions/78.html)
+* https://downloads.veracode.com/securityscan/cwe/v4/java/78.html#example