diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll b/java/ql/src/semmle/code/java/dataflow/FlowSources.qll index d2ddceec05f..ca48d583d18 100644 --- a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/src/semmle/code/java/dataflow/FlowSources.qll @@ -286,7 +286,7 @@ private class RemoteTaintedMethod extends Method { private class PlayRequestGetMethod extends Method { PlayRequestGetMethod() { - this.getDeclaringType() instanceof PlayMVCHTTPRequestHeader and + this.getDeclaringType() instanceof PlayMvcHttpRequestHeader and this.hasName(["queryString", "getQueryString", "header", "getHeader"]) } } diff --git a/java/ql/src/semmle/code/java/frameworks/play/Play.qll b/java/ql/src/semmle/code/java/frameworks/play/Play.qll index 573e66f1fb9..701766a4e75 100644 --- a/java/ql/src/semmle/code/java/frameworks/play/Play.qll +++ b/java/ql/src/semmle/code/java/frameworks/play/Play.qll @@ -1,52 +1,48 @@ +/** + * Provides classes and predicates for working with the `play` package. + */ + import java /** - * Play MVC Framework Result Class + * A `play.mvc.Result` class. */ -class PlayMVCResultClass extends Class { - PlayMVCResultClass() { this.hasQualifiedName("play.mvc", "Result") } +class PlayMvcResultClass extends Class { + PlayMvcResultClass() { this.hasQualifiedName("play.mvc", "Result") } } /** - * Play MVC Framework Results Class - * - * Documentation: https://www.playframework.com/documentation/2.8.x/JavaActions + * A `play.mvc.Results` class. */ -class PlayMVCResultsClass extends Class { - PlayMVCResultsClass() { this.hasQualifiedName("play.mvc", "Results") } +class PlayMvcResultsClass extends Class { + PlayMvcResultsClass() { this.hasQualifiedName("play.mvc", "Results") } } /** - * Play MVC Framework HTTP Request Header Class + * A `play.mvc.Http$RequestHeader` class. */ -class PlayMVCHTTPRequestHeader extends RefType { - PlayMVCHTTPRequestHeader() { this.hasQualifiedName("play.mvc", "Http$RequestHeader") } +class PlayMvcHttpRequestHeader extends RefType { + PlayMvcHttpRequestHeader() { this.hasQualifiedName("play.mvc", "Http$RequestHeader") } } /** - * Play Framework Explicit Body Parser Annotation - * - * Documentation: https://www.playframework.com/documentation/2.8.x/JavaBodyParsers#Choosing-an-explicit-body-parser + * A `play.mvc.BodyParser<>$Of"` annotation. */ class PlayBodyParserAnnotation extends Annotation { PlayBodyParserAnnotation() { this.getType().hasQualifiedName("play.mvc", "BodyParser<>$Of") } } /** - * Play Framework AddCSRFToken Annotation - * - * Documentation: https://www.playframework.com/documentation/2.8.x/JavaCsrf + * A `play.filters.csrf.AddCSRFToken` annotation. */ -class PlayAddCSRFTokenAnnotation extends Annotation { - PlayAddCSRFTokenAnnotation() { +class PlayAddCsrfTokenAnnotation extends Annotation { + PlayAddCsrfTokenAnnotation() { this.getType().hasQualifiedName("play.filters.csrf", "AddCSRFToken") } } /** - * Play Framework Async Promise - Gets the Promise Generic Member/Type of (play.libs.F) - * - * Documentation: https://www.playframework.com/documentation/2.5.1/api/java/play/libs/F.Promise.html + * A member with qualified name `F.Promise` of package `play.libs.F`. */ class PlayAsyncResultPromise extends Member { PlayAsyncResultPromise() { @@ -59,9 +55,7 @@ class PlayAsyncResultPromise extends Member { } /** - * Play Framework Async Generic Result - Gets the CompletionStage Generic Type of (java.util.concurrent) - * - * Documentation: https://www.playframework.com/documentation/2.6.x/JavaAsync + * A type with qualified name `CompletionStage` of package `java.util.concurrent`. */ class PlayAsyncResultCompletionStage extends Type { PlayAsyncResultCompletionStage() { @@ -71,7 +65,7 @@ class PlayAsyncResultCompletionStage extends Type { } /** - * Play Framework Controllers which extends PlayMVCController recursively - Used to find all Controllers + * A class which extends PlayMvcController recursively to find all controllers. */ class PlayController extends Class { PlayController() { @@ -80,9 +74,9 @@ class PlayController extends Class { } /** - * Play Framework Controller Action Methods - Mappings to route files + * A method to find PlayFramework controller action methods, these are mapping's to route files. * - * Sample Route - `POST /login @com.company.Application.login()` + * Sample Route - `POST /login @com.company.Application.login()`. * * Example - class get's `index` & `login` as valid action methods. * ``` @@ -96,24 +90,22 @@ class PlayController extends Class { * } * } * ``` - * - * Documentation: https://www.playframework.com/documentation/2.8.x/JavaActions */ class PlayControllerActionMethod extends Method { PlayControllerActionMethod() { this = any(PlayController c).getAMethod() and ( this.getReturnType() instanceof PlayAsyncResultPromise or - this.getReturnType() instanceof PlayMVCResultClass or + this.getReturnType() instanceof PlayMvcResultClass or this.getReturnType() instanceof PlayAsyncResultCompletionStage ) } } /** - * Play Action-Method parameters. These are a source of user input + * The PlayFramework action method parameters, these are a source of user input. * - * Example - Class get's `username` & `password` as valid parameters + * Example - `username` & `password` are marked as valid parameters. * ``` * public class Application extends Controller { * public Result index(String username, String password) { @@ -132,15 +124,13 @@ class PlayActionMethodQueryParameter extends Parameter { } /** - * Play Framework HTTPRequestHeader Methods - `headers`, `getQueryString`, `getHeader` - * - * Documentation: https://www.playframework.com/documentation/2.6.0/api/java/play/mvc/Http.RequestHeader.html + * A PlayFramework HttpRequestHeader method, some of these are `headers`, `getQueryString`, `getHeader`. */ -class PlayMVCHTTPRequestHeaderMethods extends Method { - PlayMVCHTTPRequestHeaderMethods() { this.getDeclaringType() instanceof PlayMVCHTTPRequestHeader } +class PlayMvcHttpRequestHeaderMethods extends Method { + PlayMvcHttpRequestHeaderMethods() { this.getDeclaringType() instanceof PlayMvcHttpRequestHeader } /** - * Gets all references to play.mvc.HTTP.RequestHeader `getQueryString` method + * A reference to the `getQueryString` method. */ MethodAccess getAQueryStringAccess() { this.hasName("getQueryString") and result = this.getAReference() @@ -148,20 +138,18 @@ class PlayMVCHTTPRequestHeaderMethods extends Method { } /** - * Play Framework mvc.Results Methods - `ok`, `status`, `redirect` - * - * Documentation: https://www.playframework.com/documentation/2.5.8/api/java/play/mvc/Results.html + * A PlayFramework results method, some of these are `ok`, `status`, `redirect`. */ -class PlayMVCResultsMethods extends Method { - PlayMVCResultsMethods() { this.getDeclaringType() instanceof PlayMVCResultsClass } +class PlayMvcResultsMethods extends Method { + PlayMvcResultsMethods() { this.getDeclaringType() instanceof PlayMvcResultsClass } /** - * Gets all references to play.mvc.Results `ok` method + * A reference to the play.mvc.Results `ok` method. */ MethodAccess getAnOkAccess() { this.hasName("ok") and result = this.getAReference() } /** - * Gets all references to play.mvc.Results `redirect` method + * A reference to the play.mvc.Results `redirect` method. */ MethodAccess getARedirectAccess() { this.hasName("redirect") and result = this.getAReference() } } diff --git a/java/ql/test/library-tests/dataflow/taintsources/PlayResource.java b/java/ql/test/library-tests/dataflow/taintsources/PlayResource.java index 547aad11edf..fac12f534ce 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/PlayResource.java +++ b/java/ql/test/library-tests/dataflow/taintsources/PlayResource.java @@ -1,34 +1,36 @@ +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import play.filters.csrf.AddCSRFToken; +import play.libs.F; +import play.mvc.BodyParser; import play.mvc.Controller; import play.mvc.Http.*; import play.mvc.Result; -import play.filters.csrf.AddCSRFToken; -import play.libs.F; -import java.util.concurrent.CompletionStage; - public class PlayResource extends Controller { - public Result index(String username, String password) { - String append_token = "password" + password; - return ok("Working"); - } + @AddCSRFToken + public Result index() { + response().setHeader("X-Play-QL", "1"); + return ok("It works!"); + } - public Result session_redirect_me() { - String url = request().getQueryString("url"); - redirect(url); - } + @BodyParser.Of() + public Result session_redirect_me(String uri) { + String url = request().getQueryString("url"); + return redirect(url); + } - public F.Promise async_promise(String token) { - ok(token); - } + public F.Promise async_promise(String token) { + return F.Promise.pure(ok(token)); + } - public CompletionStage async_completionstage(String complete) { - String return_code = "complete" + complete; - ok("Async completion Stage"); - } + public CompletionStage async_completionstage(String uri) { + return CompletableFuture.completedFuture(ok("Async completion Stage")); + } - public String not_playactionmethod(String no_action) { - String return_code = no_action; - return return_code; - } + public String not_playactionmethod(String no_action) { + String return_code = no_action; + return return_code; + } } diff --git a/java/ql/test/library-tests/dataflow/taintsources/remote.expected b/java/ql/test/library-tests/dataflow/taintsources/remote.expected index f14dac0917b..3b8bdf4e5a0 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/remote.expected +++ b/java/ql/test/library-tests/dataflow/taintsources/remote.expected @@ -23,17 +23,12 @@ | IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:33:20:33:33 | getIntent(...) | | IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:33:20:33:55 | getStringExtra(...) | | IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:34:29:34:35 | trouble | -| PlayResource.java:11:25:11:39 | username | PlayResource.java:11:25:11:39 | username | -| PlayResource.java:11:42:11:56 | password | PlayResource.java:11:42:11:56 | password | -| PlayResource.java:11:42:11:56 | password | PlayResource.java:12:31:12:51 | ... + ... | -| PlayResource.java:11:42:11:56 | password | PlayResource.java:12:44:12:51 | password | -| PlayResource.java:17:22:17:52 | getQueryString(...) | PlayResource.java:17:22:17:52 | getQueryString(...) | -| PlayResource.java:21:44:21:55 | token | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:261:27:261:40 | content | -| PlayResource.java:21:44:21:55 | token | PlayResource.java:21:44:21:55 | token | -| PlayResource.java:21:44:21:55 | token | PlayResource.java:22:12:22:16 | token | -| PlayResource.java:25:58:25:72 | complete | PlayResource.java:25:58:25:72 | complete | -| PlayResource.java:25:58:25:72 | complete | PlayResource.java:26:30:26:50 | ... + ... | -| PlayResource.java:25:58:25:72 | complete | PlayResource.java:26:43:26:50 | complete | +| PlayResource.java:19:37:19:46 | uri | PlayResource.java:19:37:19:46 | uri | +| PlayResource.java:20:18:20:48 | getQueryString(...) | PlayResource.java:20:18:20:48 | getQueryString(...) | +| PlayResource.java:24:42:24:53 | token | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:261:27:261:40 | content | +| PlayResource.java:24:42:24:53 | token | PlayResource.java:24:42:24:53 | token | +| PlayResource.java:24:42:24:53 | token | PlayResource.java:25:30:25:34 | token | +| PlayResource.java:28:56:28:65 | uri | PlayResource.java:28:56:28:65 | uri | | RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:4:30:4:40 | path | | RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:5:20:5:31 | ... + ... | | RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:5:28:5:31 | path | diff --git a/java/ql/test/library-tests/frameworks/play/PlayAddCSRFTokenAnnotation.ql b/java/ql/test/library-tests/frameworks/play/PlayAddCSRFTokenAnnotation.ql index 38fca8f6ab0..9448e4cce4e 100644 --- a/java/ql/test/library-tests/frameworks/play/PlayAddCSRFTokenAnnotation.ql +++ b/java/ql/test/library-tests/frameworks/play/PlayAddCSRFTokenAnnotation.ql @@ -1,4 +1,4 @@ import semmle.code.java.frameworks.play.Play -from PlayAddCSRFTokenAnnotation token +from PlayAddCsrfTokenAnnotation token select token diff --git a/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.expected b/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.expected new file mode 100644 index 00000000000..06f021740be --- /dev/null +++ b/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.expected @@ -0,0 +1 @@ +| resources/Resource.java:12:3:12:15 | AddCSRFToken | diff --git a/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql b/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql new file mode 100644 index 00000000000..9448e4cce4e --- /dev/null +++ b/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql @@ -0,0 +1,4 @@ +import semmle.code.java.frameworks.play.Play + +from PlayAddCsrfTokenAnnotation token +select token diff --git a/java/ql/test/library-tests/frameworks/play/PlayMVCHTTPRequestHeader.ql b/java/ql/test/library-tests/frameworks/play/PlayMVCHTTPRequestHeader.ql index 0bfb7afe537..40af3a9e43d 100644 --- a/java/ql/test/library-tests/frameworks/play/PlayMVCHTTPRequestHeader.ql +++ b/java/ql/test/library-tests/frameworks/play/PlayMVCHTTPRequestHeader.ql @@ -1,4 +1,4 @@ import semmle.code.java.frameworks.play.Play -from PlayMVCHTTPRequestHeader c +from PlayMvcHttpRequestHeader c select c.getQualifiedName(), c.getAMethod().getQualifiedName() diff --git a/java/ql/test/library-tests/frameworks/play/PlayMVCResultClass.ql b/java/ql/test/library-tests/frameworks/play/PlayMVCResultClass.ql index a924a5bf730..49bc7b9c1f9 100644 --- a/java/ql/test/library-tests/frameworks/play/PlayMVCResultClass.ql +++ b/java/ql/test/library-tests/frameworks/play/PlayMVCResultClass.ql @@ -1,4 +1,4 @@ import semmle.code.java.frameworks.play.Play -from PlayMVCResultClass m +from PlayMvcResultClass m select m.getQualifiedName() diff --git a/java/ql/test/library-tests/frameworks/play/PlayMVCResultsClass.ql b/java/ql/test/library-tests/frameworks/play/PlayMVCResultsClass.ql index 0ca501da260..4d111c08e87 100644 --- a/java/ql/test/library-tests/frameworks/play/PlayMVCResultsClass.ql +++ b/java/ql/test/library-tests/frameworks/play/PlayMVCResultsClass.ql @@ -1,4 +1,4 @@ import semmle.code.java.frameworks.play.Play -from PlayMVCResultsClass m +from PlayMvcResultsClass m select m.getQualifiedName(), m.getAMethod().getQualifiedName() diff --git a/java/ql/test/library-tests/frameworks/play/PlayMVCResultsMethods.ql b/java/ql/test/library-tests/frameworks/play/PlayMVCResultsMethods.ql index fc9f465253f..d4f274e96a6 100644 --- a/java/ql/test/library-tests/frameworks/play/PlayMVCResultsMethods.ql +++ b/java/ql/test/library-tests/frameworks/play/PlayMVCResultsMethods.ql @@ -1,4 +1,4 @@ import semmle.code.java.frameworks.play.Play -from PlayMVCResultsMethods m +from PlayMvcResultsMethods m select m.getAnOkAccess()