Spring content types: recognise constant content-type strings

Author: smowton

java/ql/lib/semmle/code/java/frameworks/spring/SpringController.qll

@@ -121,9 +121,18 @@ class SpringRequestMappingMethod extends SpringControllerMethod {
   SpringRequestMappingParameter getARequestParameter() { result = getAParameter() }
 
   /** Gets the "produces" @RequestMapping annotation value, if present. */
+  Expr getProducesExpr() { result = requestMappingAnnotation.getValue("produces") }
+
+  /** Gets the "produces" @RequestMapping annotation value, if present. */
+  Expr getAProducesExpr() {
+    result = this.getProducesExpr() and not result instanceof ArrayInit
+    or
+    result = this.getProducesExpr().(ArrayInit).getAnInit()
+  }
+
+  /** Gets the "produces" @RequestMapping annotation value, if present and a string constant. */
   string getProduces() {
-    result =
-      requestMappingAnnotation.getValue("produces").(CompileTimeConstantExpr).getStringValue()
+    result = this.getProducesExpr().(CompileTimeConstantExpr).getStringValue()
   }
 
   /** Gets the "value" @RequestMapping annotation value, if present. */

java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll

@@ -145,14 +145,19 @@ private class SpringHttpFlowStep extends SummaryModelCsv {
   }
 }
 
+private predicate specifiesContentType(SpringRequestMappingMethod method) {
+  method.getProducesExpr().(ArrayInit).getSize() != 0 or
+  not method.getProducesExpr() instanceof ArrayInit
+}
+
 private class SpringXssSink extends XSS::XssSink {
   SpringXssSink() {
     exists(SpringRequestMappingMethod requestMappingMethod, ReturnStmt rs |
       requestMappingMethod = rs.getEnclosingCallable() and
       this.asExpr() = rs.getResult() and
       (
-        not exists(requestMappingMethod.getProduces()) or
-        requestMappingMethod.getProduces().matches("text/%")
+        not specifiesContentType(requestMappingMethod) or
+        isXssVulnerableContentTypeExpr(requestMappingMethod.getAProducesExpr())
       )
     |
       // If a Spring request mapping method is either annotated with @ResponseBody (or equivalent),
@@ -251,6 +256,11 @@ private string getSpringConstantContentType(FieldAccess e) {
   )
 }
 
+private predicate isXssVulnerableContentTypeExpr(Expr e) {
+  XSS::isXssVulnerableContentType(e.(CompileTimeConstantExpr).getStringValue()) or
+  XSS::isXssVulnerableContentType(getSpringConstantContentType(e))
+}
+
 private predicate isXssSafeContentTypeExpr(Expr e) {
   XSS::isXssSafeContentType(e.(CompileTimeConstantExpr).getStringValue()) or
   XSS::isXssSafeContentType(getSpringConstantContentType(e))

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

@@ -60,7 +60,7 @@ public static ResponseEntity<String> methodContentTypeSafeStringLiteral(String u
 
   @GetMapping(value = "/xyz", produces = MediaType.TEXT_HTML_VALUE)
   public static ResponseEntity<String> methodContentTypeUnsafe(String userControlled) {
-    return ResponseEntity.ok(userControlled); // $MISSING: xss
+    return ResponseEntity.ok(userControlled); // $xss
   }
 
   @GetMapping(value = "/xyz", produces = "text/html")