Allow blacklist sanitizers.

Author: Sebastian Bauersfeld

java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql

@@ -31,10 +31,16 @@ class ResponseSplittingConfig extends TaintTracking::Configuration {
     or
     node.getType() instanceof BoxedType
     or
-    exists(MethodAccess ma |
-      ma.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
-      ma.getArgument(0).(StringLiteral).getValue().matches("%[^%") and
-      node.asExpr() = ma
+    exists(MethodAccess ma, string methodName, CompileTimeConstantExpr target |
+      node.asExpr() = ma and
+      ma.getMethod().hasQualifiedName("java.lang", "String", methodName) and
+      target = ma.getArgument(0) and
+      (
+        methodName = "replace" and target.getIntValue() = [10, 13]
+        or
+        methodName = "replaceAll" and
+        target.getStringValue().regexpMatch(".*([\n\r]|\\[\\^[^\\]\r\n]*\\]).*")
+      )
     )
   }
 }

java/ql/test/query-tests/security/CWE-113/semmle/tests/ResponseSplitting.expected

@@ -1,14 +1,20 @@
 edges
 | ResponseSplitting.java:22:20:22:67 | new Cookie(...) : Cookie | ResponseSplitting.java:23:23:23:28 | cookie |
 | ResponseSplitting.java:22:39:22:66 | getParameter(...) : String | ResponseSplitting.java:22:20:22:67 | new Cookie(...) : Cookie |
+| ResponseSplitting.java:53:14:53:48 | getParameter(...) : String | ResponseSplitting.java:59:27:59:27 | t : String |
+| ResponseSplitting.java:59:27:59:27 | t : String | ResponseSplitting.java:59:27:59:57 | replaceFirst(...) |
 nodes
 | ResponseSplitting.java:22:20:22:67 | new Cookie(...) : Cookie | semmle.label | new Cookie(...) : Cookie |
 | ResponseSplitting.java:22:39:22:66 | getParameter(...) : String | semmle.label | getParameter(...) : String |
 | ResponseSplitting.java:23:23:23:28 | cookie | semmle.label | cookie |
 | ResponseSplitting.java:28:38:28:72 | getParameter(...) | semmle.label | getParameter(...) |
 | ResponseSplitting.java:29:38:29:72 | getParameter(...) | semmle.label | getParameter(...) |
+| ResponseSplitting.java:53:14:53:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| ResponseSplitting.java:59:27:59:27 | t : String | semmle.label | t : String |
+| ResponseSplitting.java:59:27:59:57 | replaceFirst(...) | semmle.label | replaceFirst(...) |
 subpaths
 #select
 | ResponseSplitting.java:23:23:23:28 | cookie | ResponseSplitting.java:22:39:22:66 | getParameter(...) : String | ResponseSplitting.java:23:23:23:28 | cookie | Response-splitting vulnerability due to this $@. | ResponseSplitting.java:22:39:22:66 | getParameter(...) | user-provided value |
 | ResponseSplitting.java:28:38:28:72 | getParameter(...) | ResponseSplitting.java:28:38:28:72 | getParameter(...) | ResponseSplitting.java:28:38:28:72 | getParameter(...) | Response-splitting vulnerability due to this $@. | ResponseSplitting.java:28:38:28:72 | getParameter(...) | user-provided value |
 | ResponseSplitting.java:29:38:29:72 | getParameter(...) | ResponseSplitting.java:29:38:29:72 | getParameter(...) | ResponseSplitting.java:29:38:29:72 | getParameter(...) | Response-splitting vulnerability due to this $@. | ResponseSplitting.java:29:38:29:72 | getParameter(...) | user-provided value |
+| ResponseSplitting.java:59:27:59:57 | replaceFirst(...) | ResponseSplitting.java:53:14:53:48 | getParameter(...) : String | ResponseSplitting.java:59:27:59:57 | replaceFirst(...) | Response-splitting vulnerability due to this $@. | ResponseSplitting.java:53:14:53:48 | getParameter(...) | user-provided value |

java/ql/test/query-tests/security/CWE-113/semmle/tests/ResponseSplitting.java

@@ -48,4 +48,32 @@ public void addCookieName(HttpServletResponse response, Cookie cookie) {
 		Cookie cookie2 = new Cookie("name", cookie.getName());
 		response.addCookie(cookie2);
 	}
+
+	public void sanitizerTests(HttpServletRequest request, HttpServletResponse response){
+		String t = request.getParameter("contentType");
+
+		// GOOD: whitelist-based sanitization
+		response.setHeader("h", t.replaceAll("[^a-zA-Z]", ""));
+
+		// BAD: not replacing all problematic characters
+		response.setHeader("h", t.replaceFirst("[^a-zA-Z]", ""));
+
+		// GOOD: replace all line breaks
+		response.setHeader("h", t.replace('\n', ' ').replace('\r', ' '));
+
+		// FALSE NEGATIVE: replace only some line breaks
+		response.setHeader("h", t.replace('\n', ' '));
+
+		// FALSE NEGATIVE: replace only some line breaks
+		response.setHeader("h", t.replaceAll("\r", ""));
+
+		// GOOD: replace all linebreaks with a simple regex
+		response.setHeader("h", t.replaceAll("\n", "").replaceAll("\r", ""));
+
+		// GOOD: replace all linebreaks with a complex regex
+		response.setHeader("h", t.replaceAll("[\n\r]", ""));
+
+		// GOOD: replace all linebreaks with a complex regex
+		response.setHeader("h", t.replaceAll("something|[a\nb\rc]+|somethingelse", ""));
+	}
 }