diff --git a/java/ql/src/semmle/code/java/frameworks/JaxWS.qll b/java/ql/src/semmle/code/java/frameworks/JaxWS.qll index fa031e557fd..72623017926 100644 --- a/java/ql/src/semmle/code/java/frameworks/JaxWS.qll +++ b/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) { diff --git a/java/ql/src/semmle/code/java/security/XSS.qll b/java/ql/src/semmle/code/java/security/XSS.qll index 471dd8a9124..14f10cad9c8 100644 --- a/java/ql/src/semmle/code/java/security/XSS.qll +++ b/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" - ) } } diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql index eb34aa89812..721d46672e9 100644 --- a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql +++ b/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 = "" + ) } } diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java index a050d8b3873..271cba9b52c 100644 --- a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java +++ b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java @@ -30,7 +30,8 @@ public class JaxRs1 { // $RootResourceClass } @GET - void Get() { // $ResourceMethod $ResourceMethodOnResourceClass + int Get() { // $ResourceMethod $ResourceMethodOnResourceClass + return 0; // $XssSink } @POST @@ -39,7 +40,8 @@ public class JaxRs1 { // $RootResourceClass @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 @@ public class JaxRs1 { // $RootResourceClass NonRootResourceClass subResourceLocator() { // $SubResourceLocator return null; } -} -class NonRootResourceClass { // $NonRootResourceClass - @Path("") - AnotherNonRootResourceClass subResourceLocator1() { // $SubResourceLocator - return null; - } + public class NonRootResourceClass { // $NonRootResourceClass + @GET + int Get() { // $ResourceMethod $ResourceMethodOnResourceClass + return 0; // $XssSink + } - @GET - @Path("") - NotAResourceClass1 NotASubResourceLocator1() { // $ResourceMethod - return null; - } + @Produces("text/html") // $ProducesAnnotation=text/html + @POST + boolean Post() { // $ResourceMethod=text/html $ResourceMethodOnResourceClass + return false; + } - @GET - NotAResourceClass2 NotASubResourceLocator2() { // $ResourceMethod - return null; - } + @Produces(MediaType.TEXT_PLAIN) // $ProducesAnnotation=text/plain + @DELETE + double Delete() { // $ResourceMethod=text/plain $ResourceMethodOnResourceClass + return 0.0; // $XssSink + } - NotAResourceClass2 NotASubResourceLocator3() { - return null; + @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 @@ class ExtendsJaxRs1 extends JaxRs1 { } @Override - void Delete() { // $ResourceMethod=text/plain + double Delete() { // $ResourceMethod=text/plain + return 1.0; } @Override @@ -151,7 +172,8 @@ class ExtendsJaxRs1 extends JaxRs1 { @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 @@ class ExtendsJaxRs1WithProducesAnnotation extends JaxRs1 { } @Override - void Delete() { // $ResourceMethod=text/plain + double Delete() { // $ResourceMethod=text/plain + return 2.0; } @Override