Move Jax XSS sinks to JaxWS.qll and add tests

Author: owen-mc

java/ql/src/semmle/code/java/frameworks/JaxWS.qll

@@ -5,6 +5,7 @@
 
 import java
 private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.security.XSS
 
 /**
  * Gets a name for the root package of JAX-RS.
@@ -308,6 +309,21 @@ class JaxRSConsumesAnnotation extends JaxRSAnnotation {
   JaxRSConsumesAnnotation() { this.getType().hasQualifiedName(getAJaxRsPackage(), "Consumes") }
 }
 
+/** A default sink representing methods susceptible to XSS attacks. */
+private class JaxRSXssSink extends XssSink {
+  JaxRSXssSink() {
+    exists(JaxRsResourceMethod resourceMethod, ReturnStmt rs |
+      resourceMethod = any(JaxRsResourceClass resourceClass).getAResourceMethod() and
+      rs.getEnclosingCallable() = resourceMethod and
+      this.asExpr() = rs.getResult()
+    |
+      not exists(resourceMethod.getProducesAnnotation())
+      or
+      resourceMethod.getProducesAnnotation().getADeclaredContentType() = "text/plain"
+    )
+  }
+}
+
 /** A URL redirection sink from JAX-RS */
 private class JaxRsUrlRedirectSink extends SinkModelCsv {
   override predicate row(string row) {

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

@@ -1,7 +1,6 @@
 /** Provides classes to reason about Cross-site scripting (XSS) vulnerabilities. */
 
 import java
-import semmle.code.java.frameworks.JaxWS
 import semmle.code.java.frameworks.Servlets
 import semmle.code.java.frameworks.android.WebView
 import semmle.code.java.frameworks.spring.SpringController
@@ -94,16 +93,6 @@ private class DefaultXssSink extends XssSink {
         returnType instanceof RawClass
       )
     )
-    or
-    exists(JaxRsResourceMethod resourceMethod, ReturnStmt rs |
-      resourceMethod = any(JaxRsResourceClass resourceClass).getAResourceMethod() and
-      rs.getEnclosingCallable() = resourceMethod and
-      this.asExpr() = rs.getResult()
-    |
-      not exists(resourceMethod.getProducesAnnotation())
-      or
-      resourceMethod.getProducesAnnotation().getADeclaredContentType() = "text/plain"
-    )
   }
 }
 

java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql

@@ -1,5 +1,6 @@
 import java
 import semmle.code.java.frameworks.JaxWS
+import semmle.code.java.security.XSS
 import TestUtilities.InlineExpectationsTest
 
 class JaxRsTest extends InlineExpectationsTest {
@@ -143,5 +144,12 @@ class JaxRsTest extends InlineExpectationsTest {
       element = consumesAnnotation.toString() and
       value = ""
     )
+    or
+    tag = "XssSink" and
+    exists(XssSink xssSink |
+      xssSink.getLocation() = location and
+      element = xssSink.toString() and
+      value = ""
+    )
   }
 }

java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java

@@ -30,7 +30,8 @@ public JaxRs1() { // $InjectableConstructor
   }
 
   @GET
-  void Get() { // $ResourceMethod $ResourceMethodOnResourceClass
+  int Get() { // $ResourceMethod $ResourceMethodOnResourceClass
+    return 0; // $XssSink
   }
 
   @POST
@@ -39,7 +40,8 @@ void Post() { // $ResourceMethod $ResourceMethodOnResourceClass
 
   @Produces("text/plain") // $ProducesAnnotation=text/plain
   @DELETE
-  void Delete() { // $ResourceMethod=text/plain $ResourceMethodOnResourceClass
+  double Delete() { // $ResourceMethod=text/plain $ResourceMethodOnResourceClass
+    return 0.0; // $XssSink
   }
 
   @Produces(MediaType.TEXT_HTML) // $ProducesAnnotation=text/html
@@ -59,27 +61,44 @@ void Head() { // $ResourceMethod $ResourceMethodOnResourceClass
   NonRootResourceClass subResourceLocator() { // $SubResourceLocator
     return null;
   }
-}
-
-class NonRootResourceClass { // $NonRootResourceClass
-  @Path("")
-  AnotherNonRootResourceClass subResourceLocator1() { // $SubResourceLocator
-    return null;
-  }
 
-  @GET
-  @Path("")
-  NotAResourceClass1 NotASubResourceLocator1() { // $ResourceMethod
-    return null;
-  }
-
-  @GET
-  NotAResourceClass2 NotASubResourceLocator2() { // $ResourceMethod
-    return null;
-  }
-
-  NotAResourceClass2 NotASubResourceLocator3() {
-    return null;
+  public class NonRootResourceClass { // $NonRootResourceClass
+    @GET
+    int Get() { // $ResourceMethod $ResourceMethodOnResourceClass
+      return 0; // $XssSink
+    }
+
+    @Produces("text/html") // $ProducesAnnotation=text/html
+    @POST
+    boolean Post() { // $ResourceMethod=text/html $ResourceMethodOnResourceClass
+      return false;
+    }
+
+    @Produces(MediaType.TEXT_PLAIN) // $ProducesAnnotation=text/plain
+    @DELETE
+    double Delete() { // $ResourceMethod=text/plain $ResourceMethodOnResourceClass
+      return 0.0; // $XssSink
+    }
+
+    @Path("")
+    AnotherNonRootResourceClass subResourceLocator1() { // $SubResourceLocator
+      return null;
+    }
+
+    @GET
+    @Path("")
+    NotAResourceClass1 NotASubResourceLocator1() { // $ResourceMethod $ResourceMethodOnResourceClass
+      return null; // $XssSink
+    }
+
+    @GET
+    NotAResourceClass2 NotASubResourceLocator2() { // $ResourceMethod $ResourceMethodOnResourceClass
+      return null; // $XssSink
+    }
+
+    NotAResourceClass2 NotASubResourceLocator3() {
+      return null;
+    }
   }
 }
 
@@ -120,7 +139,8 @@ class NotAResourceClass2 {
 
 class ExtendsJaxRs1 extends JaxRs1 {
   @Override
-  void Get() { // $ResourceMethod
+  int Get() { // $ResourceMethod
+    return 1;
   }
 
   @Override
@@ -129,7 +149,8 @@ void Post() {
   }
 
   @Override
-  void Delete() { // $ResourceMethod=text/plain
+  double Delete() { // $ResourceMethod=text/plain
+    return 1.0;
   }
 
   @Override
@@ -151,7 +172,8 @@ void Head() {
 @Produces(MediaType.TEXT_XML) // $ProducesAnnotation=text/xml
 class ExtendsJaxRs1WithProducesAnnotation extends JaxRs1 {
   @Override
-  void Get() { // $ResourceMethod=text/xml
+  int Get() { // $ResourceMethod=text/xml
+    return 2;
   }
 
   @Override
@@ -160,7 +182,8 @@ void Post() {
   }
 
   @Override
-  void Delete() { // $ResourceMethod=text/plain
+  double Delete() { // $ResourceMethod=text/plain
+    return 2.0;
   }
 
   @Override