Move Spring XSS sink definition into SpringHttp.qll

Author: smowton

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

@@ -4,8 +4,10 @@
  */
 
 import java
+
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.frameworks.spring.SpringController
 private import semmle.code.java.security.XSS as XSS
 
 /** The class `org.springframework.http.HttpEntity` or an instantiation of it. */
@@ -143,6 +145,52 @@ private class SpringHttpFlowStep extends SummaryModelCsv {
   }
 }
 
+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/%")
+      )
+    |
+      // If a Spring request mapping method is either annotated with @ResponseBody (or equivalent),
+      // or returns a HttpEntity or sub-type, then the return value of the method is converted into
+      // a HTTP reponse using a HttpMessageConverter implementation. The implementation is chosen
+      // based on the return type of the method, and the Accept header of the request.
+      //
+      // By default, the only message converter which produces a response which is vulnerable to
+      // XSS is the StringHttpMessageConverter, which "Accept"s all text/* content types, including
+      // text/html. Therefore, if a browser request includes "text/html" in the "Accept" header,
+      // any String returned will be converted into a text/html response.
+      requestMappingMethod.isResponseBody() and
+      requestMappingMethod.getReturnType() instanceof TypeString
+      or
+      exists(Type returnType |
+        // A return type of HttpEntity<T> or ResponseEntity<T> represents an HTTP response with both
+        // a body and a set of headers. The body is subject to the same HttpMessageConverter
+        // process as above.
+        returnType = requestMappingMethod.getReturnType() and
+        (
+          returnType instanceof SpringHttpEntity
+          or
+          returnType instanceof SpringResponseEntity
+        )
+      |
+        // The type argument, representing the type of the body, is type String
+        returnType.(ParameterizedClass).getTypeArgument(0) instanceof TypeString
+        or
+        // Return type is a Raw class, which means no static type information on the body. In this
+        // case we will still treat this as an XSS sink, but rely on our taint flow steps for
+        // HttpEntity/ResponseEntity to only pass taint into those instances if the body type was
+        // String.
+        returnType instanceof RawClass
+      )
+    )
+  }
+}
+
 private string getSpringConstantContentType(FieldAccess e) {
   e.getQualifier().getType().(RefType).hasQualifiedName("org.springframework.http", "MediaType") and
   exists(string fieldName | e.getField().hasName(fieldName) |

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

@@ -45,48 +45,6 @@ private class DefaultXssSink extends XssSink {
       writer.hasFlowToExpr(ma.getQualifier()) and
       this.asExpr() = ma.getArgument(_)
     )
-    or
-    exists(SpringRequestMappingMethod requestMappingMethod, ReturnStmt rs |
-      requestMappingMethod = rs.getEnclosingCallable() and
-      this.asExpr() = rs.getResult() and
-      (
-        not exists(requestMappingMethod.getProduces()) or
-        requestMappingMethod.getProduces().matches("text/%")
-      )
-    |
-      // If a Spring request mapping method is either annotated with @ResponseBody (or equivalent),
-      // or returns a HttpEntity or sub-type, then the return value of the method is converted into
-      // a HTTP reponse using a HttpMessageConverter implementation. The implementation is chosen
-      // based on the return type of the method, and the Accept header of the request.
-      //
-      // By default, the only message converter which produces a response which is vulnerable to
-      // XSS is the StringHttpMessageConverter, which "Accept"s all text/* content types, including
-      // text/html. Therefore, if a browser request includes "text/html" in the "Accept" header,
-      // any String returned will be converted into a text/html response.
-      requestMappingMethod.isResponseBody() and
-      requestMappingMethod.getReturnType() instanceof TypeString
-      or
-      exists(Type returnType |
-        // A return type of HttpEntity<T> or ResponseEntity<T> represents an HTTP response with both
-        // a body and a set of headers. The body is subject to the same HttpMessageConverter
-        // process as above.
-        returnType = requestMappingMethod.getReturnType() and
-        (
-          returnType instanceof SpringHttpEntity
-          or
-          returnType instanceof SpringResponseEntity
-        )
-      |
-        // The type argument, representing the type of the body, is type String
-        returnType.(ParameterizedClass).getTypeArgument(0) instanceof TypeString
-        or
-        // Return type is a Raw class, which means no static type information on the body. In this
-        // case we will still treat this as an XSS sink, but rely on our taint flow steps for
-        // HttpEntity/ResponseEntity to only pass taint into those instances if the body type was
-        // String.
-        returnType instanceof RawClass
-      )
-    )
   }
 }