Merge pull request #4 from veracode/bugfix/fix-second-order-injection

Fix second order injection
veracode · Oct 24, 2019 · f90ae98 · f90ae98
2 parents b03781c + d5c2b21
commit f90ae98
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 19 deletions.
diff --git a/docs/flaws/cwe-89-second-order-sql-injection.md b/docs/flaws/cwe-89-second-order-sql-injection.md
@@ -10,10 +10,8 @@ in an SQL query, so that if I create a user with username 'test"),(1,"admin was
 
 Exploit
 -------
-** TODO: UPDATEME, this now works with the blab_name **
-**WARNING: STEP 1 will generate a 500 error, that's okay, just ignore it**
 1. Register an account with the following data:
-   Username: test"),(1,"admin was hacked
+   Username: test"),("admin","admin was hacked
    Password: test
    Confirm Password: test
    Real Name: Ms SQL Hacker
@@ -36,4 +34,4 @@ Query the data using prepared statements.
 Resources
 ---------
 * [CWE 89](https://cwe.mitre.org/data/definitions/89.html)
-* [Wikipedia: SQL injection: Second order SQL injection](https://en.wikipedia.org/wiki/SQL_injection#Second_order_SQL_injection)
+* [Wikipedia: SQL injection: Second order SQL injection](https://en.wikipedia.org/wiki/SQL_injection#Second_order_SQL_injection)
diff --git a/src/main/java/com/veracode/verademo/controller/UserController.java b/src/main/java/com/veracode/verademo/controller/UserController.java
@@ -229,22 +229,22 @@ public String processLogin(
 		logger.info("Redirecting to view: " + nextView);
 		return nextView;
 	}
-	
+
 	@RequestMapping(value = "/password-hint", method = RequestMethod.GET)
 	@ResponseBody
 	public String showPasswordHint(String username)
 	{
 		logger.info("Entering password-hint with username: " + username);
-		
+
 		if (username == null || username.isEmpty()) {
 			return "No username provided, please type in your username first";
 		}
 
 		try {
 			Class.forName("com.mysql.jdbc.Driver");
-			
+
 			Connection connect = DriverManager.getConnection(Constants.create().getJdbcConnectionString());
-	
+
 			String sql = "SELECT password_hint FROM users WHERE username = '" + username + "'";
 			logger.info(sql);
 			Statement statement = connect.createStatement();
@@ -255,7 +255,7 @@ public String showPasswordHint(String username)
 				logger.info(formatString);
 				return String.format(
 						formatString,
-						password, 
+						password,
 						String.format("%0" + (password.length() - 2) + "d", 0).replace("0", "*")
 				);
 			}
@@ -267,7 +267,7 @@ public String showPasswordHint(String username)
 		} catch (SQLException e) {
 			e.printStackTrace();
 		}
-		
+
 		return "ERROR!";
 	}
 
@@ -382,6 +382,7 @@ public String processRegisterFinish(
 
 			sqlStatement = connect.createStatement();
 			sqlStatement.execute(query.toString());
+			logger.info(query.toString());
 			/* END BAD CODE */
 
 			emailUser(username);
@@ -642,13 +643,13 @@ public String processProfile(
 		// Update user profile image
 		if (file != null && !file.isEmpty()) {
 			String imageDir = context.getRealPath("/resources/images") + File.separator;
-			
+
 			// Get old image name, if any, to delete
 			String oldImage = getProfileImageNameFromUsername(username);
 			if (oldImage != null) {
 				new File(imageDir + oldImage).delete();
 			}
-			
+
 			// TODO: check if file is png first
 			try {
 				String extension = file.getOriginalFilename().substring(file.getOriginalFilename().lastIndexOf("."));
@@ -749,7 +750,7 @@ public String downloadImage(
 
 	/**
 	 * Check if the username already exists
-	 * 
+	 *
 	 * @param username
 	 *            The username to check
 	 * @return true if the username exists, false otherwise
@@ -805,7 +806,7 @@ private boolean usernameExists(String username)
 
 	/**
 	 * Change the user's username. Since the username is the DB key, we have a lot to do
-	 * 
+	 *
 	 * @param oldUsername
 	 *            Prior username
 	 * @param newUsername
@@ -902,7 +903,7 @@ public boolean accept(File dir, String name) {
 		        return name.startsWith(username + ".");
 		    }
 		});
-		
+
 		if (matchingFiles.length < 1) {
 			return null;
 		}
@@ -951,7 +952,7 @@ public void emailExceptionsToAdmin(Throwable t)
 			mex.printStackTrace();
 		}
 	}
-	
+
 	private static String md5(String val)
 	{
 		MessageDigest md;
@@ -965,7 +966,7 @@ private static String md5(String val)
 		catch (NoSuchAlgorithmException e) {
 			e.printStackTrace();
 		}
-		
+
 		return ret;
 	}
 }
diff --git a/src/main/webapp/WEB-INF/views/blabbers.jsp b/src/main/webapp/WEB-INF/views/blabbers.jsp
@@ -94,7 +94,7 @@
 								<%
 									@SuppressWarnings("unchecked")
 									ArrayList<Blabber> blabbers = (ArrayList<Blabber>) request.getAttribute("blabbers");
-									
+
 									for (Blabber blabber : blabbers) {
 								%>
 								<tr>
@@ -117,7 +117,7 @@
 										<form class="form-inline" role="form" method="POST"
 											action="blabbers">
 											<input type="hidden" name="blabberUsername"
-												value="<%= blabber.getUsername() %>" />
+												value="<%= HtmlUtils.htmlEscape(blabber.getUsername()) %>" />
 											<input type="hidden" name="command"
 												value="<%=(blabber.getNumberListening() == 1 ? "ignore" : "listen")%>" />
 											<input type="submit" class="btn btn-default pull-right" name="button"