Jax-RS: implement content-type tracking

This follows content-type specifications across Variant-related functions and the ResponseBuilder class in order to sanitize or sink entities as appropriate.

Author: smowton

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

@@ -284,7 +284,8 @@ class MessageBodyReaderRead extends Method {
 }
 
 private string getContentTypeString(Expr e) {
-  result = e.(CompileTimeConstantExpr).getStringValue()
+  result = e.(CompileTimeConstantExpr).getStringValue() and
+  result != ""
   or
   exists(Field jaxMediaType |
     // Accesses to static fields on `MediaType` class do not have constant strings in the database
@@ -327,8 +328,9 @@ private class JaxRSXssSink extends XssSink {
     |
       not exists(resourceMethod.getProducesAnnotation())
       or
-      getContentTypeString(resourceMethod.getProducesAnnotation().getADeclaredContentTypeExpr()) =
-        "text/plain"
+      isXssVulnerableContentType(getContentTypeString(resourceMethod
+              .getProducesAnnotation()
+              .getADeclaredContentTypeExpr()))
     )
   }
 }
@@ -805,3 +807,150 @@ private class JaxRsUrlOpenSink extends SinkModelCsv {
       ]
   }
 }
+
+private predicate isXssVulnerableContentTypeExpr(Expr e) {
+  isXssVulnerableContentType(getContentTypeString(e))
+}
+
+private predicate isXssSafeContentTypeExpr(Expr e) { isXssSafeContentType(getContentTypeString(e)) }
+
+/**
+ * Gets a builder expression or related type that is configured to use the given `contentType`.
+ *
+ * This could be an instance of `Response.ResponseBuilder`, `Variant`, `Variant.VariantListBuilder` or
+ * a `List<Variant>`.
+ *
+ * This routine is used to search forwards for response entities set after the content-type is configured.
+ * It does not need to consider cases where the entity is set in the same call, or the entity has already
+ * been set: these are handled by simple sanitization below.
+ */
+private DataFlow::Node getABuilderWithExplicitContentType(Expr contentType) {
+  // Base case: ResponseBuilder.type(contentType)
+  result.asExpr() =
+    any(MethodAccess ma |
+      ma.getCallee().hasQualifiedName(getAJaxRsPackage("core"), "Response$ResponseBuilder", "type") and
+      contentType = ma.getArgument(0)
+    )
+  or
+  // Base case: new Variant(contentType, ...)
+  result.asExpr() =
+    any(ClassInstanceExpr cie |
+      cie.getConstructedType().hasQualifiedName(getAJaxRsPackage("core"), "Variant") and
+      contentType = cie.getArgument(0)
+    )
+  or
+  // Base case: Variant[.VariantListBuilder].mediaTypes(...)
+  result.asExpr() =
+    any(MethodAccess ma |
+      ma.getCallee()
+          .hasQualifiedName(getAJaxRsPackage("core"), ["Variant", "Variant$VariantListBuilder"],
+            "mediaTypes") and
+      contentType = ma.getAnArgument()
+    )
+  or
+  // Recursive case: propagate through variant list building:
+  result.asExpr() =
+    any(MethodAccess ma |
+      (
+        ma.getType()
+            .(RefType)
+            .hasQualifiedName(getAJaxRsPackage("core"), "Variant$VariantListBuilder")
+        or
+        ma.getMethod()
+            .hasQualifiedName(getAJaxRsPackage("core"), "Variant$VariantListBuilder", "build")
+      ) and
+      [ma.getAnArgument(), ma.getQualifier()] =
+        getABuilderWithExplicitContentType(contentType).asExpr()
+    )
+  or
+  // Recursive case: propagate through a List.get operation
+  result.asExpr() =
+    any(MethodAccess ma |
+      ma.getMethod().hasQualifiedName("java.util", "List<Variant>", "get") and
+      ma.getQualifier() = getABuilderWithExplicitContentType(contentType).asExpr()
+    )
+  or
+  // Recursive case: propagate through Response.ResponseBuilder operations, including the `variant(...)` operation.
+  result.asExpr() =
+    any(MethodAccess ma |
+      ma.getType().(RefType).hasQualifiedName(getAJaxRsPackage("core"), "Response$ResponseBuilder") and
+      [ma.getQualifier(), ma.getArgument(0)] =
+        getABuilderWithExplicitContentType(contentType).asExpr()
+    )
+  or
+  // Recursive case: ordinary local dataflow
+  DataFlow::localFlow(getABuilderWithExplicitContentType(contentType), result)
+}
+
+private DataFlow::Node getASanitizedBuilder() {
+  result = getABuilderWithExplicitContentType(any(Expr e | isXssSafeContentTypeExpr(e)))
+}
+
+private DataFlow::Node getAVulnerableBuilder() {
+  result = getABuilderWithExplicitContentType(any(Expr e | isXssVulnerableContentTypeExpr(e)))
+}
+
+/**
+ * A response builder sanitized by setting a safe content type.
+ *
+ * The content type could be set before the `entity(...)` call that needs sanitizing
+ * (e.g. `Response.ok().type("application/json").entity(sanitizeMe)`)
+ * or at the same time (e.g. `Response.ok(sanitizeMe, "application/json")`
+ * or the content-type could be set afterwards (e.g. `Response.ok().entity(userControlled).type("application/json")`)
+ *
+ * This differs from `getASanitizedBuilder` in that we also include functions that must set the entity
+ * at the same time, or the entity must already have been set, so propagating forwards to sanitize future
+ * build steps is not necessary.
+ */
+private class SanitizedResponseBuilder extends XssSanitizer {
+  SanitizedResponseBuilder() {
+    // e.g. sanitizeMe.type("application/json")
+    this = getASanitizedBuilder()
+    or
+    this.asExpr() =
+      any(MethodAccess ma |
+        ma.getMethod().hasQualifiedName(getAJaxRsPackage("core"), "Response", "ok") and
+        (
+          // e.g. Response.ok(sanitizeMe, new Variant("application/json", ...))
+          ma.getArgument(1) = getASanitizedBuilder().asExpr()
+          or
+          // e.g. Response.ok(sanitizeMe, "application/json")
+          isXssSafeContentTypeExpr(ma.getArgument(1))
+        )
+      )
+  }
+}
+
+/**
+ * An entity call that serves as a sink and barrier because it has a vulnerable content-type set.
+ *
+ * We flag these as direct sinks because otherwise it may be sanitized when it reaches a resource
+ * method with a safe-looking `@Produces` annotation. They are barriers because otherwise if the
+ * resource method does *not* have a safe-looking `@Produces` annotation then it would be doubly
+ * reported, once at the `entity(...)` call and once on return from the resource method.
+ */
+private class VulnerableEntity extends XssSinkBarrier {
+  VulnerableEntity() {
+    this.asExpr() =
+      any(MethodAccess ma |
+        (
+          // Vulnerable content-type already set:
+          ma.getQualifier() = getAVulnerableBuilder().asExpr()
+          or
+          // Vulnerable content-type set in the future:
+          getAVulnerableBuilder().asExpr().(MethodAccess).getQualifier*() = ma
+        ) and
+        ma.getMethod().hasName("entity")
+      ).getArgument(0)
+    or
+    this.asExpr() =
+      any(MethodAccess ma |
+        (
+          isXssVulnerableContentTypeExpr(ma.getArgument(1))
+          or
+          ma.getArgument(1) = getAVulnerableBuilder().asExpr()
+        ) and
+        ma.getMethod().hasName("ok")
+      ).getArgument(0)
+  }
+}

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

@@ -63,39 +63,43 @@ public static Response specificContentTypeSetterMethods(int route, boolean safeC
     if(safeContentType) {
       if(route == 0) {
         // via ok, as a string literal:
-        return Response.ok(userControlled, "application/json").build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, "application/json").build();
       }
       else if(route == 1) {
         // via ok, as a string constant:
-        return Response.ok(userControlled, MediaType.APPLICATION_JSON).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, MediaType.APPLICATION_JSON).build();
       }
       else if(route == 2) {
         // via ok, as a MediaType constant:
-        return Response.ok(userControlled, MediaType.APPLICATION_JSON_TYPE).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, MediaType.APPLICATION_JSON_TYPE).build();
       }
       else if(route == 3) {
         // via ok, as a Variant, via constructor:
-        return Response.ok(userControlled, new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build();
       }
       else if(route == 4) {
         // via ok, as a Variant, via static method:
-        return Response.ok(userControlled, Variant.mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, Variant.mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build();
+      }
+      else if(route == -4) {
+        // via ok, as a Variant, via static method (testing multiple media types):
+        return Response.ok(userControlled, Variant.mediaTypes(MediaType.APPLICATION_JSON_TYPE, MediaType.APPLICATION_OCTET_STREAM_TYPE).build().get(0)).build();
       }
       else if(route == 5) {
         // via ok, as a Variant, via instance method:
-        return Response.ok(userControlled, Variant.languages(Locale.UK).mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled, Variant.languages(Locale.UK).mediaTypes(MediaType.APPLICATION_JSON_TYPE).build().get(0)).build();
       }
       else if(route == 6) {
         // via builder variant, before entity:
-        return Response.ok().variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).entity(userControlled).build(); // $SPURIOUS: xss
+        return Response.ok().variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).entity(userControlled).build();
       }
       else if(route == 7) {
         // via builder variant, after entity:
-        return Response.ok().entity(userControlled).variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build(); // $SPURIOUS: xss
+        return Response.ok().entity(userControlled).variant(new Variant(MediaType.APPLICATION_JSON_TYPE, "language", "encoding")).build();
       }
       else if(route == 8) {
         // provide entity via ok, then content-type via builder:
-        return Response.ok(userControlled).type(MediaType.APPLICATION_JSON_TYPE).build(); // $SPURIOUS: xss
+        return Response.ok(userControlled).type(MediaType.APPLICATION_JSON_TYPE).build();
       }
     }
     else {
@@ -158,27 +162,27 @@ public static Response methodContentTypeSafeStringLiteral(String userControlled)
 
   @GET @Produces(MediaType.TEXT_HTML)
   public static Response methodContentTypeUnsafe(String userControlled) {
-    return Response.ok(userControlled).build(); // $MISSING: xss
+    return Response.ok(userControlled).build(); // $xss
   }
 
   @POST @Produces(MediaType.TEXT_HTML)
   public static Response methodContentTypeUnsafePost(String userControlled) {
-    return Response.ok(userControlled).build(); // $MISSING: xss
+    return Response.ok(userControlled).build(); // $xss
   }
 
   @GET @Produces("text/html")
   public static Response methodContentTypeUnsafeStringLiteral(String userControlled) {
-    return Response.ok(userControlled).build(); // $MISSING: xss
+    return Response.ok(userControlled).build(); // $xss
   }
 
   @GET @Produces({MediaType.TEXT_HTML, MediaType.APPLICATION_JSON})
   public static Response methodContentTypeMaybeSafe(String userControlled) {
-    return Response.ok(userControlled).build(); // $MISSING: xss
+    return Response.ok(userControlled).build(); // $xss
   }
 
   @GET @Produces(MediaType.APPLICATION_JSON)
   public static Response methodContentTypeSafeOverriddenWithUnsafe(String userControlled) {
-    return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $MISSING: xss
+    return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
   }
 
   @GET @Produces(MediaType.TEXT_HTML)
@@ -201,12 +205,12 @@ public String testDirectReturn(String userControlled) {
 
     @GET @Produces({"text/html"})
     public Response overridesWithUnsafe(String userControlled) {
-      return Response.ok(userControlled).build(); // $MISSING: xss
+      return Response.ok(userControlled).build(); // $xss
     }
 
     @GET
     public Response overridesWithUnsafe2(String userControlled) {
-      return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $MISSING: xss
+      return Response.ok().type(MediaType.TEXT_HTML).entity(userControlled).build(); // $xss
     }
   }
 
@@ -215,12 +219,12 @@ public Response overridesWithUnsafe2(String userControlled) {
   public static class ClassContentTypeUnsafe {
     @GET
     public Response test(String userControlled) {
-      return Response.ok(userControlled).build(); // $MISSING: xss
+      return Response.ok(userControlled).build(); // $xss
     }
 
     @GET
     public String testDirectReturn(String userControlled) {
-      return userControlled; // $MISSING: xss
+      return userControlled; // $xss
     }
 
     @GET @Produces({"application/json"})