Fix XSS FPs when content type is safe

Author: owen-mc

java/ql/lib/semmle/code/java/frameworks/Servlets.qll

@@ -315,6 +315,16 @@ class ResponseSetHeaderMethod extends Method {
   }
 }
 
+/**
+ * The method `setContentType` declared in `javax.servlet.http.HttpServletResponse`.
+ */
+class ResponseSetContentTypeMethod extends Method {
+  ResponseSetContentTypeMethod() {
+    this.getDeclaringType() instanceof ServletResponse and
+    this.hasName("setContentType")
+  }
+}
+
 /**
  * A class that has `javax.servlet.Servlet` as an ancestor.
  */

java/ql/lib/semmle/code/java/security/XSS.qll

@@ -92,9 +92,17 @@ private class WritingMethod extends Method {
 /** An output stream or writer that writes to a servlet, JSP or JSF response. */
 class XssVulnerableWriterSource extends MethodCall {
   XssVulnerableWriterSource() {
-    this.getMethod() instanceof ServletResponseGetWriterMethod
-    or
-    this.getMethod() instanceof ServletResponseGetOutputStreamMethod
+    (
+      this.getMethod() instanceof ServletResponseGetWriterMethod
+      or
+      this.getMethod() instanceof ServletResponseGetOutputStreamMethod
+    ) and
+    not exists(MethodCall mc |
+      mc.getMethod() instanceof ResponseSetContentTypeMethod and
+      isXssSafeContentTypeString(mc.getArgument(0).(CompileTimeConstantExpr).getStringValue())
+    |
+      DataFlow::localExprFlow(mc.getQualifier(), this.getQualifier())
+    )
     or
     exists(Method m | m = this.getMethod() |
       m.hasQualifiedName("javax.servlet.jsp", "JspContext", "getOut")
@@ -106,6 +114,11 @@ class XssVulnerableWriterSource extends MethodCall {
   }
 }
 
+pragma[nomagic]
+private predicate isXssSafeContentTypeString(string s) {
+  s = any(CompileTimeConstantExpr cte).getStringValue() and isXssSafeContentType(s)
+}
+
 /**
  * A xss vulnerable writer source node.
  */

java/ql/test/query-tests/security/CWE-079/semmle/tests/XSS.java

@@ -43,12 +43,12 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response, b
 			if(getWriter) {
 				// GOOD: set content-type to something safe
 				response.setContentType("text/plain");
-				response.getWriter().print(request.getPathInfo()); // $ SPURIOUS: xss
+				response.getWriter().print(request.getPathInfo());
 			}
 			else {
 				// GOOD: set content-type to something safe
 				response.setContentType("text/plain");
-				response.getOutputStream().write(request.getPathInfo()); // $ SPURIOUS: xss
+				response.getOutputStream().write(request.getPathInfo().getBytes());
 			}
 		}
 		else {
@@ -60,7 +60,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response, b
 			else {
 				// BAD: set content-type to something that is not safe
 				response.setContentType("text/html");
-				response.getOutputStream().write(request.getPathInfo()); // $ xss
+				response.getOutputStream().write(request.getPathInfo().getBytes()); // $ xss
 			}
 		}
 	}