Initial support for Java - Play Framework > 2.6.x by torque59 · Pull Request #4299 · github/codeql

torque59 · 2020-09-18T11:38:46Z

Initial Support for Play Framework (2.6.x)

This should support majority of the Play functionality - request controllers, response methods, query parameters, csrf-token etc. for 2.6.x.
Adds Play query parameter sources to RemoteFlowSource and some of the methods to RemoteTaintedMethod
Tests Included (for taint flow only).

I couldn't find much guidelines on how to contribute a framework, mostly the structure was stubbed out of Spring.

N.B: Haven't checked for full compatibility on 2.8.x and other latest etc, but should work.

Let me know if this looks good !!

smowton

Looks good; some suggestions and comments. Am I right understanding this currently only covers sources of untrusted information? In order to measure the effectiveness of the models it would be best to have some sinks to cover as well -- for example, https://www.playframework.com/documentation/1.2/security#xss their template engine normally escapes things for us, but use of the raw method allows us to bypass that, or https://stackoverflow.com/questions/14743199/how-to-pass-raw-html-to-play-framework-view shows some other ways a lazy dev might bypass the sanitizing template engine :)

We can go ahead as-is if you prefer, but including at least one common attack sink would likely make this a lot more productive for not much effort.

java/ql/src/semmle/code/java/frameworks/play/PlayMVCResults.qll

java/ql/src/semmle/code/java/frameworks/play/PlayAddCSRFToken.qll

java/ql/src/semmle/code/java/frameworks/play/PlayHTTPRequestHeader.qll

java/ql/src/semmle/code/java/frameworks/play/PlayBodyParser.qll

java/ql/src/semmle/code/java/dataflow/FlowSources.qll

torque59

Thank you for taking the time to review this. @smowton Like said this covers mostly sources. Play depends a lot of its basic security on its configuration, direct security impact sinks as such are less likely within the framework.
I will have a check on the raw method, as well as any other possible sinks and how its being used in the version 2.x and above and probably raise it as a different PR.

@smowton there was a mistake with a blind git add done, hence the couple of bad git conflict markers in commits, apologies for the same.

java/ql/src/semmle/code/java/dataflow/FlowSources.qll

java/ql/src/semmle/code/java/frameworks/play/PlayAddCSRFToken.qll

java/ql/src/semmle/code/java/frameworks/play/PlayBodyParser.qll

java/ql/src/semmle/code/java/frameworks/play/PlayHTTPRequestHeader.qll

smowton

Some more simplifications, plus I think rather than have lots of small files with a class or two each, let's put them into a single Play.qll for now, then we can split it as and when it becomes ungainly.

Regarding testing, let me know if you decide to implement or punt on the sinks, then I'll start a test job.

java/ql/src/semmle/code/java/frameworks/play/PlayAddCSRFToken.qll

java/ql/src/semmle/code/java/frameworks/play/PlayHTTPRequestHeader.qll

java/ql/src/semmle/code/java/frameworks/play/PlayController.qll

…d methods to remotetainted method for play

smowton

There are some parts of this PR that are untested still:

Test by checking these are recognised as sources, as is already done in the taintsources/remote.expected file:

Parameters of an action method that returns a Result promise
Parameters of an action method that returns a Result CompletionStage
(negative test: add a method that returns some other type and check it is not wrongly detected)
The header getQueryString method

Then regarding any of these classes that are not currently contributing to a RemoteFlowSource, such as everything to do with the Results class, PlayAddCSRFTokenAnnotation, PlayBodyParserAnnotation and so on, create a simple test under https://github.com/github/codeql/tree/main/java/ql/test/library-tests/frameworks that does something like from PlayAddCSRFTokenAnnotation ann select ann, just to check they are indeed binding the right classes / methods.

As a rule, make sure everything that is proposed for addition is covered by some test.

java/ql/src/semmle/code/java/dataflow/FlowSources.qll

java/ql/src/semmle/code/java/frameworks/play/Play.qll

java/ql/src/semmle/code/java/dataflow/FlowSources.qll

torque59 · 2021-01-06T17:46:45Z

@smowton This should contain most of the test required for the PlayFramework, including the remoteflowsources. Fixed the other suggestions as mentioned.

smowton · 2021-01-07T11:41:41Z

Thanks for the additional tests! Passing this on to get a code-owner review

aschackmull · 2021-02-02T13:33:12Z

It doesn't look like the tests compile.

torque59 · 2021-02-02T16:44:48Z

It doesn't look like the tests compile.

@aschackmull Can you let me know which ones are failing ?

All of them pass for me with following command below.

codeql test run -v --search-path=ql/ ql/test/library-tests/frameworks/play/

java/ql/test/library-tests/frameworks/play/resources/Resource.java

java/ql/src/semmle/code/java/frameworks/play/Play.qll

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>

java/ql/src/semmle/code/java/frameworks/play/Play.qll

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>

java/ql/src/semmle/code/java/frameworks/play/Play.qll

aschackmull · 2021-02-25T09:42:43Z

Looks like there might be compilation errors or something like it in the tests:

Some tests failed!
    ql/java/ql/test/library-tests/dataflow/taintsources/local.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/dataflow/taintsources/remote.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayController.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql: Extractor error (Extraction failed)

torque59 · 2021-02-25T11:14:59Z

Looks like there might be compilation errors or something like it in the tests:

Some tests failed!
    ql/java/ql/test/library-tests/dataflow/taintsources/local.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/dataflow/taintsources/remote.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayController.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql: Extractor error (Extraction failed)
    ql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql: Extractor error (Extraction failed)

@aschackmull Works for me locally

codeql test run -v --show-extractor-output --search-path=ql/ ql/test/library-tests/frameworks/play/                                                                                                                                   (play-framework+18885) 16:40:11
Executing 9 tests in 1 directories.
Extracting test database in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
Running database init in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
Running legacy java test extraction in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
Finializing test database in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
Compiling queries in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql (11.1s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql (8.3s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql (6.7s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql (31.1s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql (33s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayController.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayController.ql (30.6s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql.

Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql (27s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql (8s).
Compiling query plan for /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql.
Compiled /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql (6s).
Executing tests in /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play.
[1/9 comp 11.1s eval 2.2s] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql
[2/9 comp 8.3s eval 126ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql
[3/9 comp 6.7s eval 53ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql
[4/9 comp 31.1s eval 33ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql
[5/9 comp 33s eval 144ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql
[6/9 comp 30.6s eval 54ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayController.ql
[7/9 comp 27s eval 71ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql
[8/9 comp 8s eval 54ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql
[9/9 comp 6s eval 209ms] PASSED /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql
Cleaning out existing /Users/fpattara/Desktop/Workarea/personal/codeql-play/codeql/java/ql/test/library-tests/frameworks/play/play.testproj.
All 9 tests passed.

aschackmull · 2021-02-26T13:33:09Z

Weird. It passes locally for me as well. I'll try to rerun the test.

aschackmull · 2021-03-02T08:03:46Z

A colleague of mine figured it out. There are a bunch of return statements missing in stubs/playframework-2.6.x/play/mvc/Http.java (possibly among other things), which means that the tests don't compile. Previously the flow-checks part of the java compiler were ignored in qltests, but this changed recently. The change has so far only been applied to the CI, but not yet fully integrated in the command-line tool. If your codeql is new enough you ought to be able to reproduce locally by running CODEQL_EXTRACTOR_JAVA_FLOW_CHECKS=true codeql test run.

aschackmull · 2021-03-02T08:07:36Z

java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:263: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:271: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:279: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:287: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:295: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:299: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:307: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:59: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:133: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:141: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:149: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:157: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:165: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:173: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:181: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:185: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:194: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:203: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:210: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:213: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:219: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:312: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:326: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:393: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:402: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:412: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:421: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:434: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:444: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:453: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:783: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:794: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:805: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:815: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:824: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:834: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:845: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:855: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:866: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:875: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:884: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:898: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:908: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:918: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:928: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:939: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:949: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:957: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:964: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:971: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:982: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:991: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:999: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1007: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1016: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1028: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1032: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1039: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1043: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1046: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1055: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1062: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1066: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1070: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1077: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1081: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1091: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1095: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1102: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1111: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1120: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1128: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1132: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1141: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1150: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1160: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1168: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1176: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1180: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1189: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1193: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1204: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1214: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1218: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1228: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1237: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1241: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1248: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1252: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1259: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1304: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1307: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1310: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1327: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1335: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1343: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1351: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1365: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1373: error: missing return statement
java/ql/test/stubs/playframework-2.6.x/play/mvc/Http.java:1397: error: missing return statement

torque59 · 2021-03-02T09:19:02Z

hmm @aschackmull i am still not able to reproduce with codeql 2.4.4 (latest available)

codeql version
CodeQL command-line toolchain release 2.4.4.
Copyright (C) 2019-2021 GitHub, Inc.

CODEQL_EXTRACTOR_JAVA_FLOW_CHECKS=true codeql test run --search-path=ql/ ql/test/library-tests/frameworks/play/
Executing 9 tests in 1 directories.
Extracting test database in /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play.
Compiling queries in /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play.
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql (5s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql (3.2s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql (1.9s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql (1.8s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql (2s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayController.ql (1.8s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql (1.8s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql (2s).
Compiled /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql (1.7s).
Executing tests in /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play.
[1/9 comp 5s eval 603ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayActionMethodQueryParameter.ql
[2/9 comp 3.2s eval 32ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsMethods.ql
[3/9 comp 1.9s eval 9ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayBodyParserAnnotation.ql
[4/9 comp 1.8s eval 5ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultClass.ql
[5/9 comp 2s eval 39ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcHttpRequestHeader.ql
[6/9 comp 1.8s eval 11ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayController.ql
[7/9 comp 1.8s eval 26ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayMvcResultsClass.ql
[8/9 comp 2s eval 8ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayAddCsrfTokenAnnotation.ql
[9/9 comp 1.7s eval 58ms] PASSED /Users/francis/Desktop/Workarea/personal/codeql/java/ql/test/library-tests/frameworks/play/PlayControllerActionMethod.ql
All 9 tests passed.

aschackmull · 2021-03-02T09:27:12Z

Then I guess it hasn't yet made it into a released version. The above log output with the error positions should make it easy to fix, though.

aschackmull · 2021-03-03T10:30:59Z

remote.expected now needs a slight update:

@@ -24,8 +24,10 @@
 | 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:19:37:19:46 | uri | PlayResource.java:19:37:19:46 | uri |
+| PlayResource.java:20:18:20:48 | getQueryString(...) | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:634:33:634:42 | url |
 | 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:20:18:20:48 | getQueryString(...) | PlayResource.java:21:21:21:23 | url |
+| PlayResource.java:24:42:24:53 | token | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:94:27:94: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 |

aschackmull · 2021-03-03T10:33:36Z

Could you also add a change note stating that Play Query Parameters are now recognized as remote sources of taint. See java/change-notes for examples.

torque59 requested a review from a team as a code owner September 18, 2020 11:38

github-actions bot added the Java label Sep 18, 2020

torque59 changed the title ~~Initial support for Play Framework > 2.6.x~~ Sep 18, 2020

smowton reviewed Oct 20, 2020

View reviewed changes

smowton self-assigned this Oct 20, 2020

aibaars reviewed Oct 21, 2020

View reviewed changes

java/ql/src/semmle/code/java/dataflow/FlowSources.qll Outdated Show resolved Hide resolved

java/ql/src/semmle/code/java/dataflow/FlowSources.qll Outdated Show resolved Hide resolved

java/ql/src/semmle/code/java/dataflow/FlowSources.qll Outdated Show resolved Hide resolved

torque59 commented Oct 22, 2020

View reviewed changes

torque59 added 3 commits October 22, 2020 20:20

Initial support for Play Framework > 2.6.x

ddc544a

Naming Fixes

33f7d52

Feedback incorporation and documentation updates

f7d63f8

torque59 force-pushed the play-framework branch from fa523e4 to f7d63f8 Compare October 22, 2020 14:53

torque59 added 2 commits October 22, 2020 20:27

Feedback incorporation and documentation updates

5c256da

updates

518de82

aibaars reviewed Oct 22, 2020

View reviewed changes

java/ql/src/semmle/code/java/frameworks/play/PlayHTTPRequestHeader.qll Outdated Show resolved Hide resolved

torque59 added 2 commits October 22, 2020 22:25

updates & conflict marker removal

d216dcd

Play remote source update to return functionaccessexpr

5d5b849

smowton reviewed Oct 23, 2020

View reviewed changes

feedback integration - Move all files to Play.qll, improvements to ad…

27c554c

…d methods to remotetainted method for play

torque59 requested a review from smowton October 27, 2020 06:10

smowton reviewed Oct 27, 2020

View reviewed changes

java/ql/src/semmle/code/java/dataflow/FlowSources.qll Outdated Show resolved Hide resolved

java/ql/src/semmle/code/java/frameworks/play/Play.qll Outdated Show resolved Hide resolved

java/ql/src/semmle/code/java/dataflow/FlowSources.qll Outdated Show resolved Hide resolved

torque59 added 2 commits January 6, 2021 22:57

Playframework test cases & review fixes

1f5a466

missing new lines

435502e

torque59 requested a review from smowton January 6, 2021 17:36

aschackmull reviewed Feb 3, 2021

View reviewed changes

java/ql/test/library-tests/frameworks/play/resources/Resource.java Outdated Show resolved Hide resolved

test case return statements and feedback

6832333

aschackmull requested changes Feb 15, 2021

View reviewed changes