From b111194fbcbca7f566975c06eeb09ef83b3ba05b Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 20 Sep 2024 09:25:24 +0200 Subject: [PATCH 01/15] Shared: Simplify `PrettyPrintModels.ql` --- csharp/ql/test/TestUtilities/PrettyPrintModels.ql | 4 ---- go/ql/test/TestUtilities/PrettyPrintModels.ql | 4 ---- java/ql/test/TestUtilities/PrettyPrintModels.ql | 4 ---- shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll | 2 +- 4 files changed, 1 insertion(+), 13 deletions(-) diff --git a/csharp/ql/test/TestUtilities/PrettyPrintModels.ql b/csharp/ql/test/TestUtilities/PrettyPrintModels.ql index 05e6506e6664..9d960bc4f058 100644 --- a/csharp/ql/test/TestUtilities/PrettyPrintModels.ql +++ b/csharp/ql/test/TestUtilities/PrettyPrintModels.ql @@ -5,7 +5,3 @@ import semmle.code.csharp.dataflow.internal.ExternalFlow import codeql.dataflow.test.ProvenancePathGraph import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults - -from string relation, int row, int column, string data -where results(relation, row, column, data) -select relation, row, column, data diff --git a/go/ql/test/TestUtilities/PrettyPrintModels.ql b/go/ql/test/TestUtilities/PrettyPrintModels.ql index e49f38b2b758..2981d8cb0d69 100644 --- a/go/ql/test/TestUtilities/PrettyPrintModels.ql +++ b/go/ql/test/TestUtilities/PrettyPrintModels.ql @@ -4,7 +4,3 @@ import semmle.go.dataflow.ExternalFlow import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults - -from string relation, int row, int column, string data -where results(relation, row, column, data) -select relation, row, column, data diff --git a/java/ql/test/TestUtilities/PrettyPrintModels.ql b/java/ql/test/TestUtilities/PrettyPrintModels.ql index eff62a86050e..d127de1e150a 100644 --- a/java/ql/test/TestUtilities/PrettyPrintModels.ql +++ b/java/ql/test/TestUtilities/PrettyPrintModels.ql @@ -4,7 +4,3 @@ import semmle.code.java.dataflow.ExternalFlow import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults - -from string relation, int row, int column, string data -where results(relation, row, column, data) -select relation, row, column, data diff --git a/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll b/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll index 0260b8160a39..25ae20f184ca 100644 --- a/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll +++ b/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll @@ -113,7 +113,7 @@ module TestPostProcessing { ) } - predicate results(string relation, int row, int column, string data) { + query predicate results(string relation, int row, int column, string data) { queryResults(relation, row, column, data) and (relation != "edges" or column != provenanceColumn()) or From e7a3e6bfedb652f092cfdc42745544c8c3e41ccb Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 20 Sep 2024 13:58:49 +0200 Subject: [PATCH 02/15] Shared: Post-processing query for inline test expectations --- .../util/test/InlineExpectationsTest.qll | 318 +++++++++++++++++- 1 file changed, 309 insertions(+), 9 deletions(-) diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 409634fb79aa..8bf8a7db95b6 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -134,8 +134,31 @@ module Make { * predicate for an active test will be ignored. This makes it possible to write multiple tests in * different `.ql` files that all query the same source code. */ + bindingset[result] string getARelevantTag(); + /** + * Holds if expected tag `expectedTag` matches actual tag `actualTag`. + * + * This is normally defined as `expectedTag = actualTag`. + */ + bindingset[expectedTag, actualTag] + default predicate tagMatches(string expectedTag, string actualTag) { expectedTag = actualTag } + + /** Holds if expectations marked with `expectedTag` are optional. */ + bindingset[expectedTag] + default predicate tagIsOptional(string expectedTag) { none() } + + /** + * Holds if expected value `expectedValue` matches actual value `actualValue`. + * + * This is normally defined as `expectedValue = actualValue`. + */ + bindingset[expectedValue, actualValue] + default predicate valueMatches(string expectedValue, string actualValue) { + expectedValue = actualValue + } + /** * Returns the actual results of the query that is being tested. Each result consist of the * following values: @@ -200,13 +223,13 @@ module Make { not exists(ActualTestResult actualResult | expectation.matchesActualResult(actualResult)) and expectation.getTag() = TestImpl::getARelevantTag() and element = expectation and - ( - expectation instanceof GoodTestExpectation and - message = "Missing result: " + expectation.getExpectationText() - or - expectation instanceof FalsePositiveTestExpectation and - message = "Fixed spurious result: " + expectation.getExpectationText() - ) + not expectation.isOptional() + | + expectation instanceof GoodTestExpectation and + message = "Missing result: " + expectation.getExpectationText() + or + expectation instanceof FalsePositiveTestExpectation and + message = "Fixed spurious result: " + expectation.getExpectationText() ) or exists(InvalidTestExpectation expectation | @@ -311,9 +334,11 @@ module Make { predicate matchesActualResult(ActualTestResult actualResult) { onSameLine(pragma[only_bind_into](this), actualResult) and - this.getTag() = actualResult.getTag() and - this.getValue() = actualResult.getValue() + TestImpl::tagMatches(this.getTag(), actualResult.getTag()) and + TestImpl::valueMatches(this.getValue(), actualResult.getValue()) } + + predicate isOptional() { TestImpl::tagIsOptional(tag) } } // Note: These next three classes correspond to all the possible values of type `TColumn`. @@ -337,6 +362,18 @@ module Make { string getExpectation() { result = expectation } } + /** + * Gets a test expectation that matches the actual result at the given location. + */ + ValidTestExpectation getAMatchingExpectation( + Impl::Location location, string element, string tag, string val, boolean optional + ) { + exists(ActualTestResult actualResult | + result.matchesActualResult(actualResult) and + actualResult = TActualResult(location, element, tag, val, optional) + ) + } + query predicate testFailures(FailureLocatable element, string message) { hasFailureMessage(element, message) } @@ -385,6 +422,7 @@ module Make { * ``` */ module MergeTests implements TestSig { + bindingset[result] string getARelevantTag() { result = TestImpl1::getARelevantTag() or result = TestImpl2::getARelevantTag() } @@ -408,6 +446,7 @@ module Make { module MergeTests3 implements TestSig { private module M = MergeTests, TestImpl3>; + bindingset[result] string getARelevantTag() { result = M::getARelevantTag() } predicate hasActualResult(Impl::Location location, string element, string tag, string value) { @@ -427,6 +466,7 @@ module Make { { private module M = MergeTests, TestImpl4>; + bindingset[result] string getARelevantTag() { result = M::getARelevantTag() } predicate hasActualResult(Impl::Location location, string element, string tag, string value) { @@ -448,6 +488,7 @@ module Make { private module M = MergeTests, TestImpl5>; + bindingset[result] string getARelevantTag() { result = M::getARelevantTag() } predicate hasActualResult(Impl::Location location, string element, string tag, string value) { @@ -590,3 +631,262 @@ private string expectationPattern() { result = tags + "(?:=" + value + ")?" ) } + +/** + * Provides logic for creating a `@kind test-postprocess` query that checks + * inline test expectations using `$ Alert` markers. + * + * The postprocessing query works for queries of kind `problem` and `path-problem`, + * and each query result must have a matching `$ Alert` comment. It is possible to + * augment the comment with a query ID, in order to support cases where multiple + * `.qlref` tests share the same test code: + * + * ```rust + * var x = ""; // $ Alert[rust/unused-value] + * return; + * foo(); // $ Alert[rust/unreachable-code] + * ``` + * + * In the example above, the `$ Alert[rust/unused-value]` commment is only taken + * into account in the test for the query with ID `rust/unused-value`, and vice + * versa for the `$ Alert[rust/unreachable-code]` comment. + * + * For `path-problem` queries, each source and sink must additionally be annotated + * (`$ Source` and `$ Sink`, respectively), except when their location coincides + * with the location of the alert itself, in which case only `$ Alert` is needed. + * + * Example: + * + * ```csharp + * var queryParam = Request.QueryString["param"]; // $ Source + * Write(Html.Raw(queryParam)); // $ Alert + * ``` + * + * Morover, it is possible to tag sources with a unique identifier: + * + * ```csharp + * var queryParam = Request.QueryString["param"]; // $ Source=source1 + * Write(Html.Raw(queryParam)); // $ Alert=source1 + * ``` + * + * In this case, the source and sink must have the same tag in order + * to be matched. + */ +module TestPostProcessing { + external predicate queryResults(string relation, int row, int column, string data); + + external predicate queryRelations(string relation); + + external predicate queryMetadata(string key, string value); + + private string getQueryId() { queryMetadata("id", result) } + + private string getQueryKind() { queryMetadata("kind", result) } + + signature module InputSig { + string getRelativeUrl(Input::Location location); + } + + module Make Input2> { + private import InlineExpectationsTest as InlineExpectationsTest + private import InlineExpectationsTest::Make + + /** + * Gets the tag to be used for the path-problem source at result row `row`. + * + * This is either `Source` or `Alert`, depending on whether the location + * of the source matches the location of the alert. + */ + private string getSourceTag(int row) { + getQueryKind() = "path-problem" and + exists(string loc | queryResults("#select", row, 2, loc) | + if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Source" + ) + } + + /** + * Gets the tag to be used for the path-problem sink at result row `row`. + * + * This is either `Sink` or `Alert`, depending on whether the location + * of the sink matches the location of the alert. + */ + private string getSinkTag(int row) { + getQueryKind() = "path-problem" and + exists(string loc | queryResults("#select", row, 4, loc) | + if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Sink" + ) + } + + /** + * A configuration for matching `// $ Source=foo` comments against actual + * path-problem sources. + */ + private module PathProblemSourceTestInput implements TestSig { + string getARelevantTag() { result = getSourceTag(_) } + + bindingset[expectedValue, actualValue] + predicate valueMatches(string expectedValue, string actualValue) { + exists(expectedValue) and + actualValue = "" + } + + additional predicate hasPathProblemSource( + int row, Input::Location location, string element, string tag, string value + ) { + getQueryKind() = "path-problem" and + exists(string loc | + queryResults("#select", row, 2, loc) and + queryResults("#select", row, 3, element) and + tag = getSourceTag(row) and + value = "" and + Input2::getRelativeUrl(location) = loc + ) + } + + predicate hasActualResult(Input::Location location, string element, string tag, string value) { + hasPathProblemSource(_, location, element, tag, value) + } + } + + private module PathProblemSourceTest = MakeTest; + + private module TestInput implements TestSig { + bindingset[result] + string getARelevantTag() { any() } + + private string getTagRegex() { + exists(string sourceSinkTags | + getQueryKind() = "problem" and + sourceSinkTags = "" + or + sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_) + | + result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?" + ) + } + + bindingset[expectedTag, actualTag] + predicate tagMatches(string expectedTag, string actualTag) { + actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and + ( + // expected tag is annotated with a query ID + getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3) + or + // expected tag is not annotated with a query ID + not exists(expectedTag.regexpCapture(getTagRegex(), 3)) + ) + } + + bindingset[expectedTag] + predicate tagIsOptional(string expectedTag) { + // ignore irrelevant tags + not expectedTag.regexpMatch(getTagRegex()) + or + // ignore tags annotated with a query ID that does not match the current query ID + exists(string queryId | + queryId = expectedTag.regexpCapture(getTagRegex(), 3) and + queryId != getQueryId() + ) + } + + bindingset[expectedValue, actualValue] + predicate valueMatches(string expectedValue, string actualValue) { + expectedValue = actualValue + or + actualValue = "" + } + + private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5; + + /** + * Gets the expected sink value for result row `row`. This value must + * match the value at the corresponding path-problem source (if it is + * present). + */ + private string getSinkValue(int row) { + exists(Input::Location location, string element, string tag, string val | + hasPathProblemSource(row, location, element, tag, val) and + result = + PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false) + .getValue() + ) + } + + private predicate hasPathProblemSink( + int row, Input::Location location, string element, string tag, string value + ) { + getQueryKind() = "path-problem" and + exists(string loc | + queryResults("#select", row, 4, loc) and + queryResults("#select", row, 5, element) and + tag = getSinkTag(row) and + Input2::getRelativeUrl(location) = loc + | + not exists(getSinkValue(row)) and value = "" + or + value = getSinkValue(row) + ) + } + + private predicate hasAlert(Input::Location location, string element, string tag, string value) { + getQueryKind() = ["problem", "path-problem"] and + exists(int row, string loc | + queryResults("#select", row, 0, loc) and + queryResults("#select", row, 2, element) and + tag = "Alert" and + value = "" and + Input2::getRelativeUrl(location) = loc and + not hasPathProblemSource(row, location, _, _, _) and + not hasPathProblemSink(row, location, _, _, _) + ) + } + + predicate hasActualResult(Input::Location location, string element, string tag, string value) { + hasPathProblemSource(_, location, element, tag, value) + or + hasPathProblemSink(_, location, element, tag, value) + or + hasAlert(location, element, tag, value) + } + } + + private module Test = MakeTest; + + private newtype TTestFailure = + MkTestFailure(Test::FailureLocatable f, string message) { Test::testFailures(f, message) } + + private predicate rankedTestFailures(int i, MkTestFailure f) { + f = + rank[i](MkTestFailure f0, Test::FailureLocatable fl, string message, string filename, + int startLine, int startColumn, int endLine, int endColumn | + f0 = MkTestFailure(fl, message) and + fl.getLocation().hasLocationInfo(filename, startLine, startColumn, endLine, endColumn) + | + f0 order by filename, startLine, startColumn, endLine, endColumn, message + ) + } + + query predicate results(string relation, int row, int column, string data) { + queryResults(relation, row, column, data) + or + exists(MkTestFailure f, Test::FailureLocatable fl, string message | + relation = "testFailures" and + rankedTestFailures(row, f) and + f = MkTestFailure(fl, message) + | + column = 0 and data = Input2::getRelativeUrl(fl.getLocation()) + or + column = 1 and data = fl.toString() + or + column = 2 and data = message + ) + } + + query predicate resultRelations(string relation) { + queryRelations(relation) + or + Test::testFailures(_, _) and + relation = "testFailures" + } + } +} From 8ba80fd02240c10ee79fbfb3b0d122dfe1ef2dde Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 20 Sep 2024 13:59:45 +0200 Subject: [PATCH 03/15] C#: Post-processing query for inline test expectations --- .../TestUtilities/InlineExpectationsTest.qll | 2 +- .../InlineExpectationsTestQuery.ql | 21 +++++ .../CWE-079/XSS/Index.cshtml.g.cs | 4 +- .../CWE-079/XSS/XSS.expected | 94 ++++++++++++------- .../Security Features/CWE-079/XSS/XSS.qlref | 4 +- .../CWE-079/XSS/XSSAspNet.cs | 11 ++- .../CWE-079/XSS/XSSAspNetCore.cs | 22 +++-- .../Useless Code/UnusedLabel/UnusedLabel.cs | 4 +- .../UnusedLabel/UnusedLabel.expected | 2 +- .../UnusedLabel/UnusedLabel.qlref | 3 +- 10 files changed, 111 insertions(+), 56 deletions(-) create mode 100644 csharp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll b/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll index 6faee89923cc..75f918c36a1b 100644 --- a/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll +++ b/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -1,5 +1,5 @@ /** - * Inline expectation tests for CSharp. + * Inline expectation tests for C#. * See `shared/util/codeql/util/test/InlineExpectationsTest.qll` */ diff --git a/csharp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/csharp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..35901ee64012 --- /dev/null +++ b/csharp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import csharp +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/Index.cshtml.g.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/Index.cshtml.g.cs index bbd3b9a66ff6..ea5285c596de 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/Index.cshtml.g.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/Index.cshtml.g.cs @@ -30,7 +30,7 @@ public class Pages_Index : global::Microsoft.AspNetCore.Mvc.RazorPages.Page #line 3 "Index.cshtml" ViewData["Title"] = "ASP.NET Core"; - var message = Request.Query["m"]; + var message = Request.Query["m"]; // $ Source=message #line default #line hidden @@ -38,7 +38,7 @@ public class Pages_Index : global::Microsoft.AspNetCore.Mvc.RazorPages.Page WriteLiteral("
\n
\n"); #nullable restore #line 14 "Index.cshtml" -Write(Html.Raw(message)); // BAD +Write(Html.Raw(message)); // $ Alert=message #line default #line hidden diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected index 371917cd02f1..fe184fdff75a 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected @@ -2,14 +2,14 @@ | Index.cshtml:14:16:14:22 | call to operator implicit conversion | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | Index.cshtml:14:16:14:22 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | User-provided value | | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteral() method. | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | User-provided value | | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteralTo() method. | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | User-provided value | -| XSSAspNet.cs:43:28:43:55 | access to indexer | XSSAspNet.cs:43:28:43:46 | access to property QueryString : NameValueCollection | XSSAspNet.cs:43:28:43:55 | access to indexer | $@ flows to here and is written to HTML or JavaScript. | XSSAspNet.cs:43:28:43:46 | access to property QueryString : NameValueCollection | User-provided value | -| XSSAspNetCore.cs:21:52:21:76 | call to operator implicit conversion | XSSAspNetCore.cs:21:52:21:64 | access to property Query : IQueryCollection | XSSAspNetCore.cs:21:52:21:76 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:21:52:21:64 | access to property Query : IQueryCollection | User-provided value | -| XSSAspNetCore.cs:44:51:44:53 | access to parameter foo | XSSAspNetCore.cs:40:56:40:58 | foo : String | XSSAspNetCore.cs:44:51:44:53 | access to parameter foo | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:40:56:40:58 | foo : String | User-provided value | -| XSSAspNetCore.cs:51:43:51:67 | access to property Value | XSSAspNetCore.cs:51:43:51:67 | access to property Value | XSSAspNetCore.cs:51:43:51:67 | access to property Value | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:51:43:51:67 | access to property Value | User-provided value | -| XSSAspNetCore.cs:58:43:58:73 | call to method ToString | XSSAspNetCore.cs:58:43:58:55 | access to property Query : IQueryCollection | XSSAspNetCore.cs:58:43:58:73 | call to method ToString | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:58:43:58:55 | access to property Query : IQueryCollection | User-provided value | -| XSSAspNetCore.cs:61:44:61:66 | access to indexer | XSSAspNetCore.cs:61:44:61:56 | access to property Query : IQueryCollection | XSSAspNetCore.cs:61:44:61:66 | access to indexer | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:61:44:61:56 | access to property Query : IQueryCollection | User-provided value | -| XSSAspNetCore.cs:69:43:69:61 | access to property ContentType | XSSAspNetCore.cs:69:43:69:61 | access to property ContentType | XSSAspNetCore.cs:69:43:69:61 | access to property ContentType | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:69:43:69:61 | access to property ContentType | User-provided value | -| XSSAspNetCore.cs:72:51:72:72 | call to operator implicit conversion | XSSAspNetCore.cs:72:51:72:65 | access to property Headers : IHeaderDictionary | XSSAspNetCore.cs:72:51:72:72 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:72:51:72:65 | access to property Headers : IHeaderDictionary | User-provided value | +| XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | $@ flows to here and is written to HTML or JavaScript. | XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | User-provided value | +| XSSAspNetCore.cs:22:52:22:57 | call to operator implicit conversion | XSSAspNetCore.cs:21:26:21:38 | access to property Query : IQueryCollection | XSSAspNetCore.cs:22:52:22:57 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:21:26:21:38 | access to property Query : IQueryCollection | User-provided value | +| XSSAspNetCore.cs:45:51:45:53 | access to parameter foo | XSSAspNetCore.cs:41:56:41:58 | foo : String | XSSAspNetCore.cs:45:51:45:53 | access to parameter foo | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:41:56:41:58 | foo : String | User-provided value | +| XSSAspNetCore.cs:53:43:53:46 | access to local variable req2 | XSSAspNetCore.cs:52:24:52:48 | access to property Value : String | XSSAspNetCore.cs:53:43:53:46 | access to local variable req2 | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:52:24:52:48 | access to property Value : String | User-provided value | +| XSSAspNetCore.cs:61:43:61:46 | access to local variable req3 | XSSAspNetCore.cs:60:24:60:36 | access to property Query : IQueryCollection | XSSAspNetCore.cs:61:43:61:46 | access to local variable req3 | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:60:24:60:36 | access to property Query : IQueryCollection | User-provided value | +| XSSAspNetCore.cs:65:44:65:47 | access to local variable req4 | XSSAspNetCore.cs:64:24:64:36 | access to property Query : IQueryCollection | XSSAspNetCore.cs:65:44:65:47 | access to local variable req4 | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:64:24:64:36 | access to property Query : IQueryCollection | User-provided value | +| XSSAspNetCore.cs:74:43:74:44 | access to local variable ct | XSSAspNetCore.cs:73:22:73:40 | access to property ContentType : String | XSSAspNetCore.cs:74:43:74:44 | access to local variable ct | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:73:22:73:40 | access to property ContentType : String | User-provided value | +| XSSAspNetCore.cs:78:51:78:56 | call to operator implicit conversion | XSSAspNetCore.cs:77:26:77:40 | access to property Headers : IHeaderDictionary | XSSAspNetCore.cs:78:51:78:56 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSSAspNetCore.cs:77:26:77:40 | access to property Headers : IHeaderDictionary | User-provided value | edges | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | Index.cshtml:14:16:14:22 | call to operator implicit conversion | provenance | | | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | provenance | | @@ -18,17 +18,29 @@ edges | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | provenance | | | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:19:25:19:52 | access to indexer : String | provenance | MaD:3 | | XSSAspNet.cs:19:25:19:52 | access to indexer : String | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | provenance | | -| XSSAspNet.cs:43:28:43:46 | access to property QueryString : NameValueCollection | XSSAspNet.cs:43:28:43:55 | access to indexer | provenance | | -| XSSAspNet.cs:43:28:43:46 | access to property QueryString : NameValueCollection | XSSAspNet.cs:43:28:43:55 | access to indexer | provenance | MaD:3 | -| XSSAspNetCore.cs:21:52:21:64 | access to property Query : IQueryCollection | XSSAspNetCore.cs:21:52:21:76 | call to operator implicit conversion | provenance | | -| XSSAspNetCore.cs:40:56:40:58 | foo : String | XSSAspNetCore.cs:44:51:44:53 | access to parameter foo | provenance | | -| XSSAspNetCore.cs:58:43:58:55 | access to property Query : IQueryCollection | XSSAspNetCore.cs:58:43:58:62 | access to indexer : StringValues | provenance | | -| XSSAspNetCore.cs:58:43:58:62 | access to indexer : StringValues | XSSAspNetCore.cs:58:43:58:73 | call to method ToString | provenance | MaD:1 | -| XSSAspNetCore.cs:61:44:61:56 | access to property Query : IQueryCollection | XSSAspNetCore.cs:61:44:61:63 | access to indexer : StringValues | provenance | | -| XSSAspNetCore.cs:61:44:61:56 | access to property Query : IQueryCollection | XSSAspNetCore.cs:61:44:61:66 | access to indexer | provenance | | -| XSSAspNetCore.cs:61:44:61:63 | access to indexer : StringValues | XSSAspNetCore.cs:61:44:61:66 | access to indexer | provenance | | -| XSSAspNetCore.cs:61:44:61:63 | access to indexer : StringValues | XSSAspNetCore.cs:61:44:61:66 | access to indexer | provenance | MaD:2 | -| XSSAspNetCore.cs:72:51:72:65 | access to property Headers : IHeaderDictionary | XSSAspNetCore.cs:72:51:72:72 | call to operator implicit conversion | provenance | | +| XSSAspNet.cs:43:17:43:22 | access to local variable sayHi2 : String | XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | provenance | | +| XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | XSSAspNet.cs:43:17:43:22 | access to local variable sayHi2 : String | provenance | | +| XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | XSSAspNet.cs:43:26:43:53 | access to indexer : String | provenance | MaD:3 | +| XSSAspNet.cs:43:26:43:53 | access to indexer : String | XSSAspNet.cs:43:17:43:22 | access to local variable sayHi2 : String | provenance | | +| XSSAspNetCore.cs:21:17:21:22 | access to local variable source : StringValues | XSSAspNetCore.cs:22:52:22:57 | call to operator implicit conversion | provenance | | +| XSSAspNetCore.cs:21:26:21:38 | access to property Query : IQueryCollection | XSSAspNetCore.cs:21:17:21:22 | access to local variable source : StringValues | provenance | | +| XSSAspNetCore.cs:41:56:41:58 | foo : String | XSSAspNetCore.cs:45:51:45:53 | access to parameter foo | provenance | | +| XSSAspNetCore.cs:52:17:52:20 | access to local variable req2 : String | XSSAspNetCore.cs:53:43:53:46 | access to local variable req2 | provenance | | +| XSSAspNetCore.cs:52:24:52:48 | access to property Value : String | XSSAspNetCore.cs:52:17:52:20 | access to local variable req2 : String | provenance | | +| XSSAspNetCore.cs:60:17:60:20 | access to local variable req3 : String | XSSAspNetCore.cs:61:43:61:46 | access to local variable req3 | provenance | | +| XSSAspNetCore.cs:60:24:60:36 | access to property Query : IQueryCollection | XSSAspNetCore.cs:60:24:60:43 | access to indexer : StringValues | provenance | | +| XSSAspNetCore.cs:60:24:60:43 | access to indexer : StringValues | XSSAspNetCore.cs:60:24:60:54 | call to method ToString : String | provenance | MaD:1 | +| XSSAspNetCore.cs:60:24:60:54 | call to method ToString : String | XSSAspNetCore.cs:60:17:60:20 | access to local variable req3 : String | provenance | | +| XSSAspNetCore.cs:64:17:64:20 | access to local variable req4 : String | XSSAspNetCore.cs:65:44:65:47 | access to local variable req4 | provenance | | +| XSSAspNetCore.cs:64:24:64:36 | access to property Query : IQueryCollection | XSSAspNetCore.cs:64:17:64:20 | access to local variable req4 : String | provenance | | +| XSSAspNetCore.cs:64:24:64:36 | access to property Query : IQueryCollection | XSSAspNetCore.cs:64:24:64:43 | access to indexer : StringValues | provenance | | +| XSSAspNetCore.cs:64:24:64:43 | access to indexer : StringValues | XSSAspNetCore.cs:64:17:64:20 | access to local variable req4 : String | provenance | | +| XSSAspNetCore.cs:64:24:64:43 | access to indexer : StringValues | XSSAspNetCore.cs:64:24:64:46 | access to indexer : String | provenance | MaD:2 | +| XSSAspNetCore.cs:64:24:64:46 | access to indexer : String | XSSAspNetCore.cs:64:17:64:20 | access to local variable req4 : String | provenance | | +| XSSAspNetCore.cs:73:17:73:18 | access to local variable ct : String | XSSAspNetCore.cs:74:43:74:44 | access to local variable ct | provenance | | +| XSSAspNetCore.cs:73:22:73:40 | access to property ContentType : String | XSSAspNetCore.cs:73:17:73:18 | access to local variable ct : String | provenance | | +| XSSAspNetCore.cs:77:17:77:22 | access to local variable header : StringValues | XSSAspNetCore.cs:78:51:78:56 | call to operator implicit conversion | provenance | | +| XSSAspNetCore.cs:77:26:77:40 | access to property Headers : IHeaderDictionary | XSSAspNetCore.cs:77:17:77:22 | access to local variable header : StringValues | provenance | | models | 1 | Summary: Microsoft.Extensions.Primitives; StringValues; false; ToString; (); ; Argument[this]; ReturnValue; taint; manual | | 2 | Summary: Microsoft.Extensions.Primitives; StringValues; false; get_Item; (System.Int32); ; Argument[this]; ReturnValue; taint; manual | @@ -42,20 +54,32 @@ nodes | XSSAspNet.cs:19:25:19:52 | access to indexer : String | semmle.label | access to indexer : String | | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | semmle.label | access to local variable sayHi | | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | semmle.label | access to local variable sayHi | -| XSSAspNet.cs:43:28:43:46 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | -| XSSAspNet.cs:43:28:43:55 | access to indexer | semmle.label | access to indexer | -| XSSAspNetCore.cs:21:52:21:64 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | -| XSSAspNetCore.cs:21:52:21:76 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | -| XSSAspNetCore.cs:40:56:40:58 | foo : String | semmle.label | foo : String | -| XSSAspNetCore.cs:44:51:44:53 | access to parameter foo | semmle.label | access to parameter foo | -| XSSAspNetCore.cs:51:43:51:67 | access to property Value | semmle.label | access to property Value | -| XSSAspNetCore.cs:58:43:58:55 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | -| XSSAspNetCore.cs:58:43:58:62 | access to indexer : StringValues | semmle.label | access to indexer : StringValues | -| XSSAspNetCore.cs:58:43:58:73 | call to method ToString | semmle.label | call to method ToString | -| XSSAspNetCore.cs:61:44:61:56 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | -| XSSAspNetCore.cs:61:44:61:63 | access to indexer : StringValues | semmle.label | access to indexer : StringValues | -| XSSAspNetCore.cs:61:44:61:66 | access to indexer | semmle.label | access to indexer | -| XSSAspNetCore.cs:69:43:69:61 | access to property ContentType | semmle.label | access to property ContentType | -| XSSAspNetCore.cs:72:51:72:65 | access to property Headers : IHeaderDictionary | semmle.label | access to property Headers : IHeaderDictionary | -| XSSAspNetCore.cs:72:51:72:72 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | +| XSSAspNet.cs:43:17:43:22 | access to local variable sayHi2 : String | semmle.label | access to local variable sayHi2 : String | +| XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | +| XSSAspNet.cs:43:26:43:53 | access to indexer : String | semmle.label | access to indexer : String | +| XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | semmle.label | access to local variable sayHi2 | +| XSSAspNetCore.cs:21:17:21:22 | access to local variable source : StringValues | semmle.label | access to local variable source : StringValues | +| XSSAspNetCore.cs:21:26:21:38 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | +| XSSAspNetCore.cs:22:52:22:57 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | +| XSSAspNetCore.cs:41:56:41:58 | foo : String | semmle.label | foo : String | +| XSSAspNetCore.cs:45:51:45:53 | access to parameter foo | semmle.label | access to parameter foo | +| XSSAspNetCore.cs:52:17:52:20 | access to local variable req2 : String | semmle.label | access to local variable req2 : String | +| XSSAspNetCore.cs:52:24:52:48 | access to property Value : String | semmle.label | access to property Value : String | +| XSSAspNetCore.cs:53:43:53:46 | access to local variable req2 | semmle.label | access to local variable req2 | +| XSSAspNetCore.cs:60:17:60:20 | access to local variable req3 : String | semmle.label | access to local variable req3 : String | +| XSSAspNetCore.cs:60:24:60:36 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | +| XSSAspNetCore.cs:60:24:60:43 | access to indexer : StringValues | semmle.label | access to indexer : StringValues | +| XSSAspNetCore.cs:60:24:60:54 | call to method ToString : String | semmle.label | call to method ToString : String | +| XSSAspNetCore.cs:61:43:61:46 | access to local variable req3 | semmle.label | access to local variable req3 | +| XSSAspNetCore.cs:64:17:64:20 | access to local variable req4 : String | semmle.label | access to local variable req4 : String | +| XSSAspNetCore.cs:64:24:64:36 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | +| XSSAspNetCore.cs:64:24:64:43 | access to indexer : StringValues | semmle.label | access to indexer : StringValues | +| XSSAspNetCore.cs:64:24:64:46 | access to indexer : String | semmle.label | access to indexer : String | +| XSSAspNetCore.cs:65:44:65:47 | access to local variable req4 | semmle.label | access to local variable req4 | +| XSSAspNetCore.cs:73:17:73:18 | access to local variable ct : String | semmle.label | access to local variable ct : String | +| XSSAspNetCore.cs:73:22:73:40 | access to property ContentType : String | semmle.label | access to property ContentType : String | +| XSSAspNetCore.cs:74:43:74:44 | access to local variable ct | semmle.label | access to local variable ct | +| XSSAspNetCore.cs:77:17:77:22 | access to local variable header : StringValues | semmle.label | access to local variable header : StringValues | +| XSSAspNetCore.cs:77:26:77:40 | access to property Headers : IHeaderDictionary | semmle.label | access to property Headers : IHeaderDictionary | +| XSSAspNetCore.cs:78:51:78:56 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | subpaths diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.qlref b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.qlref index 15face9de9c1..df73539b55cd 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.qlref +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.qlref @@ -1,2 +1,4 @@ query: Security Features/CWE-079/XSS.ql -postprocess: TestUtilities/PrettyPrintModels.ql +postprocess: + - TestUtilities/PrettyPrintModels.ql + - TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNet.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNet.cs index c9779db5bc6c..2046aa4f3093 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNet.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNet.cs @@ -16,14 +16,14 @@ public override void Execute() { Layout = "~/_SiteLayout.cshtml"; Page.Title = "Contact"; - var sayHi = Request.QueryString["sayHi"]; + var sayHi = Request.QueryString["sayHi"]; // $ Source=sayHi if (sayHi.IsEmpty()) { WriteLiteral(""); // GOOD: hard-coded, not user input } else { - WriteLiteral(sayHi); // BAD: user input flows to HTML unencoded + WriteLiteral(sayHi); // $ Alert=sayHi WriteLiteral(HttpUtility.HtmlEncode(sayHi)); // Good: user input is encoded before it flows to HTML } @@ -33,15 +33,16 @@ public override void Execute() } else { - WriteLiteralTo(Output, sayHi); // BAD: user input flows to HTML unencoded + WriteLiteralTo(Output, sayHi); // $ Alert=sayHi WriteLiteralTo(Output, Html.Encode(sayHi)); // Good: user input is encoded before it flows to HTML } BeginContext("~/Views/Home/Contact.cshtml", 288, 32, false); Write(Html.Raw("")); // GOOD: hard-coded, not user input - Write(Html.Raw(Request.QueryString["sayHi"])); // BAD: user input flows to HTML unencoded - Write(Html.Raw(HttpContext.Current.Server.HtmlEncode(Request.QueryString["sayHi"]))); // Good: user input is encoded before it flows to HTML + var sayHi2 = Request.QueryString["sayHi"]; // $ Source=sayHi2 + Write(Html.Raw(sayHi2)); // $ Alert=sayHi2 + Write(Html.Raw(HttpContext.Current.Server.HtmlEncode(sayHi2))); // Good: user input is encoded before it flows to HTML EndContext("~/Views/Home/Contact.cshtml", 288, 32, false); } } diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNetCore.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNetCore.cs index 89d7ca98f960..2f1902d103b6 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNetCore.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNetCore.cs @@ -18,7 +18,8 @@ public IActionResult Index() { // BAD: flow of content type to. var v = new ViewResult(); - v.ViewData["BadData"] = new HtmlString(Request.Query["Bad data"]); + var source = Request.Query["Bad data"]; // $ Source=req1 + v.ViewData["BadData"] = new HtmlString(source); // $ Alert=req1 StringValues vOut; Request.Query.TryGetValue("Foo", out vOut); @@ -37,28 +38,31 @@ public IActionResult Index() [HttpPost("Test")] [ValidateAntiForgeryToken] - public IActionResult Submit([FromQuery] string foo) + public IActionResult Submit([FromQuery] string foo) // $ Source=foo { var view = new ViewResult(); //BAD: flow of submitted value to view in HtmlString. - view.ViewData["FOO"] = new HtmlString(foo); + view.ViewData["FOO"] = new HtmlString(foo); // $ Alert=foo return view; } public IActionResult IndexToModel() { //BAD: flow of submitted value to view in HtmlString. - HtmlString v = new HtmlString(Request.QueryString.Value); + var req2 = Request.QueryString.Value; // $ Source=req2 + HtmlString v = new HtmlString(req2); // $ Alert=req2 return View(new HomeViewModel() { Message = "Message from Index", Description = v }); } public IActionResult About() { //BAD: flow of submitted value to view in HtmlString. - HtmlString v = new HtmlString(Request.Query["Foo"].ToString()); + var req3 = Request.Query["Foo"].ToString(); // $ Source=req3 + HtmlString v = new HtmlString(req3); // $ Alert=req3 //BAD: flow of submitted value to view in HtmlString. - HtmlString v1 = new HtmlString(Request.Query["Foo"][0]); + var req4 = Request.Query["Foo"][0]; // $ Source=req4 + HtmlString v1 = new HtmlString(req4); // $ Alert=req4 return View(new HomeViewModel() { Message = "Message from About", Description = v }); } @@ -66,10 +70,12 @@ public IActionResult About() public IActionResult Contact() { //BAD: flow of user content type to view in HtmlString. - HtmlString v = new HtmlString(Request.ContentType); + var ct = Request.ContentType; // $ Source=ct + HtmlString v = new HtmlString(ct); // $ Alert=ct //BAD: flow of headers to view in HtmlString. - HtmlString v1 = new HtmlString(value: Request.Headers["Foo"]); + var header = Request.Headers["Foo"]; // $ Source=header + HtmlString v1 = new HtmlString(value: header); // $ Alert=header return View(new HomeViewModel() { Message = "Message from Contact", Description = v }); } diff --git a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.cs b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.cs index 15f36fde1509..96e3dde2c3a1 100644 --- a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.cs +++ b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.cs @@ -3,13 +3,13 @@ class UnusedLabelTest void F1() { goto a; - a: // GOOD + a: // GOOD ; } void F2() { - a: // BAD + a: // $ Alert ; } } diff --git a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.expected b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.expected index d203e9837c82..5cab2be7ccb6 100644 --- a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.expected +++ b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.expected @@ -1 +1 @@ -| UnusedLabel.cs:12:9:12:9 | a: | This label is not used. | +| UnusedLabel.cs:12:5:12:5 | a: | This label is not used. | diff --git a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.qlref b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.qlref index 45ab96cef50b..bbf7012c6eea 100644 --- a/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.qlref +++ b/csharp/ql/test/query-tests/Useless Code/UnusedLabel/UnusedLabel.qlref @@ -1 +1,2 @@ -Useless code/UnusedLabel.ql \ No newline at end of file +query: Useless code/UnusedLabel.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file From e2b614d18a180ef6dd91c511ad5c34019f8bfac6 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 09:41:25 +0200 Subject: [PATCH 04/15] Java: Post-processing query for inline test expectations --- .../InlineExpectationsTestQuery.ql | 21 ++ .../AndroidIntentRedirectionTest.expected | 285 +++++++++++++++++- .../CWE-940/AndroidIntentRedirectionTest.java | 104 +++---- .../CWE-940/AndroidIntentRedirectionTest.ql | 18 -- .../AndroidIntentRedirectionTest.qlref | 2 + 5 files changed, 358 insertions(+), 72 deletions(-) create mode 100644 java/ql/test/TestUtilities/InlineExpectationsTestQuery.ql delete mode 100644 java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.ql create mode 100644 java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.qlref diff --git a/java/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/java/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..b0360dfecd8d --- /dev/null +++ b/java/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import java +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.expected b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.expected index 48de9172b362..5b0588c225b1 100644 --- a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.expected +++ b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.expected @@ -1,2 +1,283 @@ -failures -testFailures +#select +| AndroidIntentRedirectionTest.java:15:25:15:45 | new Intent[] | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:15:25:15:45 | new Intent[] | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:16:25:16:45 | new Intent[] | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:16:25:16:45 | new Intent[] | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:17:23:17:28 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:17:23:17:28 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:18:23:18:28 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:18:23:18:28 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:19:29:19:34 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:19:29:19:34 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:20:31:20:36 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:20:31:20:36 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:21:32:21:37 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:21:32:21:37 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:22:32:22:37 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:22:32:22:37 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:23:38:23:43 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:23:38:23:43 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:24:38:24:43 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:24:38:24:43 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:25:38:25:43 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:25:38:25:43 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:26:38:26:43 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:26:38:26:43 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:29:22:29:27 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:29:22:29:27 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:30:28:30:33 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:30:28:30:33 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:31:32:31:37 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:31:32:31:37 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:32:23:32:28 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:32:23:32:28 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:33:23:33:28 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:33:23:33:28 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:34:29:34:34 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:34:29:34:34 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:35:29:35:34 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:35:29:35:34 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:36:46:36:51 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:36:46:36:51 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:37:29:37:34 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:37:29:37:34 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:38:35:38:40 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:38:35:38:40 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:39:36:39:41 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:39:36:39:41 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:40:42:40:47 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:40:42:40:47 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:47:27:47:32 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:47:27:47:32 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:49:27:49:32 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:49:27:49:32 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:52:27:52:32 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:52:27:52:32 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:54:27:54:32 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:54:27:54:32 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:61:27:61:32 | intent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:61:27:61:32 | intent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:69:31:69:39 | fwdIntent | AndroidIntentRedirectionTest.java:67:30:67:40 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:69:31:69:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:67:30:67:40 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:74:31:74:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:74:31:74:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:79:31:79:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:79:31:79:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:85:31:85:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:85:31:85:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:96:31:96:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:96:31:96:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:103:31:103:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:103:31:103:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:109:31:109:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:109:31:109:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:116:31:116:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:116:31:116:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:131:31:131:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:131:31:131:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:138:31:138:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:138:31:138:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:145:31:145:39 | fwdIntent | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:145:31:145:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:164:35:164:43 | fwdIntent | AndroidIntentRedirectionTest.java:161:41:161:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:164:35:164:43 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:161:41:161:51 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:173:31:173:44 | originalIntent | AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:173:31:173:44 | originalIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:193:31:193:39 | fwdIntent | AndroidIntentRedirectionTest.java:192:52:192:62 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:193:31:193:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:192:52:192:62 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:197:31:197:39 | fwdIntent | AndroidIntentRedirectionTest.java:196:53:196:63 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:197:31:197:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:196:53:196:63 | getIntent(...) | user-provided value | +| AndroidIntentRedirectionTest.java:201:31:201:39 | fwdIntent | AndroidIntentRedirectionTest.java:200:56:200:66 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:201:31:201:39 | fwdIntent | Arbitrary Android activities or services can be started from a $@. | AndroidIntentRedirectionTest.java:200:56:200:66 | getIntent(...) | user-provided value | +edges +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:15:39:15:44 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:16:39:16:44 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:17:23:17:28 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:18:23:18:28 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:19:29:19:34 | intent | provenance | Sink:MaD:228 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:20:31:20:36 | intent | provenance | Sink:MaD:3 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:21:32:21:37 | intent | provenance | Sink:MaD:4 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:22:32:22:37 | intent | provenance | Sink:MaD:5 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:23:38:23:43 | intent | provenance | Sink:MaD:6 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:24:38:24:43 | intent | provenance | Sink:MaD:7 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:25:38:25:43 | intent | provenance | Sink:MaD:7 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:26:38:26:43 | intent | provenance | Sink:MaD:7 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:29:22:29:27 | intent | provenance | Sink:MaD:233 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:30:28:30:33 | intent | provenance | Sink:MaD:234 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:31:32:31:37 | intent | provenance | Sink:MaD:232 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:32:23:32:28 | intent | provenance | Sink:MaD:219 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:33:23:33:28 | intent | provenance | Sink:MaD:219 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:34:29:34:34 | intent | provenance | Sink:MaD:220 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:35:29:35:34 | intent | provenance | Sink:MaD:220 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:36:46:36:51 | intent | provenance | Sink:MaD:221 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:37:29:37:34 | intent | provenance | Sink:MaD:222 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:38:35:38:40 | intent | provenance | Sink:MaD:223 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:39:36:39:41 | intent | provenance | Sink:MaD:224 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:40:42:40:47 | intent | provenance | Sink:MaD:225 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:47:27:47:32 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:49:27:49:32 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:52:27:52:32 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:54:27:54:32 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:61:27:61:32 | intent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:73:56:73:61 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:78:40:78:45 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:83:40:83:45 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:84:25:84:30 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:95:38:95:43 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:101:43:101:48 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:107:65:107:70 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:114:59:114:64 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:129:58:129:63 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:136:54:136:59 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | AndroidIntentRedirectionTest.java:143:25:143:30 | intent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:12:34:12:81 | getParcelableExtra(...) : Parcelable | provenance | MaD:326 | +| AndroidIntentRedirectionTest.java:12:34:12:81 | getParcelableExtra(...) : Parcelable | AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | provenance | | +| AndroidIntentRedirectionTest.java:15:25:15:45 | {...} : Intent[] [[]] : Intent | AndroidIntentRedirectionTest.java:15:25:15:45 | new Intent[] | provenance | Sink:MaD:226 | +| AndroidIntentRedirectionTest.java:15:39:15:44 | intent : Intent | AndroidIntentRedirectionTest.java:15:25:15:45 | {...} : Intent[] [[]] : Intent | provenance | | +| AndroidIntentRedirectionTest.java:16:25:16:45 | {...} : Intent[] [[]] : Intent | AndroidIntentRedirectionTest.java:16:25:16:45 | new Intent[] | provenance | Sink:MaD:226 | +| AndroidIntentRedirectionTest.java:16:39:16:44 | intent : Intent | AndroidIntentRedirectionTest.java:16:25:16:45 | {...} : Intent[] [[]] : Intent | provenance | | +| AndroidIntentRedirectionTest.java:67:30:67:40 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:67:30:67:77 | getParcelableExtra(...) : Parcelable | provenance | MaD:326 | +| AndroidIntentRedirectionTest.java:67:30:67:77 | getParcelableExtra(...) : Parcelable | AndroidIntentRedirectionTest.java:68:36:68:47 | (...)... : Intent | provenance | | +| AndroidIntentRedirectionTest.java:68:36:68:47 | (...)... : Intent | AndroidIntentRedirectionTest.java:69:31:69:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:73:17:73:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:74:31:74:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:73:56:73:61 | intent : Intent | AndroidIntentRedirectionTest.java:73:56:73:89 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:73:56:73:89 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:73:17:73:25 | fwdIntent [post update] : Intent | provenance | MaD:363 | +| AndroidIntentRedirectionTest.java:78:17:78:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:79:31:79:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:78:40:78:45 | intent : Intent | AndroidIntentRedirectionTest.java:78:40:78:75 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:78:40:78:75 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:78:17:78:25 | fwdIntent [post update] : Intent | provenance | MaD:364 | +| AndroidIntentRedirectionTest.java:83:17:83:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:85:31:85:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:83:40:83:45 | intent : Intent | AndroidIntentRedirectionTest.java:83:40:83:75 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:83:40:83:75 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:83:17:83:25 | fwdIntent [post update] : Intent | provenance | MaD:364 | +| AndroidIntentRedirectionTest.java:84:25:84:30 | intent : Intent | AndroidIntentRedirectionTest.java:84:25:84:58 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:84:25:84:58 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:83:17:83:25 | fwdIntent [post update] : Intent | provenance | MaD:364 | +| AndroidIntentRedirectionTest.java:95:17:95:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:96:31:96:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:95:38:95:43 | intent : Intent | AndroidIntentRedirectionTest.java:95:38:95:73 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:95:38:95:73 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:95:17:95:25 | fwdIntent [post update] : Intent | provenance | MaD:378 | +| AndroidIntentRedirectionTest.java:101:25:101:85 | new ComponentName(...) : ComponentName | AndroidIntentRedirectionTest.java:102:40:102:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:101:43:101:48 | intent : Intent | AndroidIntentRedirectionTest.java:101:43:101:78 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:101:43:101:78 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:101:25:101:85 | new ComponentName(...) : ComponentName | provenance | MaD:238 | +| AndroidIntentRedirectionTest.java:102:17:102:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:103:31:103:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:102:40:102:48 | component : ComponentName | AndroidIntentRedirectionTest.java:102:17:102:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:107:43:107:99 | new ComponentName(...) : ComponentName | AndroidIntentRedirectionTest.java:108:40:108:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:107:65:107:70 | intent : Intent | AndroidIntentRedirectionTest.java:107:65:107:98 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:107:65:107:98 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:107:43:107:99 | new ComponentName(...) : ComponentName | provenance | MaD:238 | +| AndroidIntentRedirectionTest.java:108:17:108:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:109:31:109:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:108:40:108:48 | component : ComponentName | AndroidIntentRedirectionTest.java:108:17:108:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:114:25:114:93 | new ComponentName(...) : ComponentName | AndroidIntentRedirectionTest.java:115:40:115:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:114:59:114:64 | intent : Intent | AndroidIntentRedirectionTest.java:114:59:114:92 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:114:59:114:92 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:114:25:114:93 | new ComponentName(...) : ComponentName | provenance | MaD:236 | +| AndroidIntentRedirectionTest.java:115:17:115:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:116:31:116:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:115:40:115:48 | component : ComponentName | AndroidIntentRedirectionTest.java:115:17:115:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:129:25:129:92 | createRelative(...) : ComponentName | AndroidIntentRedirectionTest.java:130:40:130:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:129:58:129:63 | intent : Intent | AndroidIntentRedirectionTest.java:129:58:129:91 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:129:58:129:91 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:129:25:129:92 | createRelative(...) : ComponentName | provenance | MaD:240 | +| AndroidIntentRedirectionTest.java:130:17:130:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:131:31:131:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:130:40:130:48 | component : ComponentName | AndroidIntentRedirectionTest.java:130:17:130:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:136:25:136:94 | createRelative(...) : ComponentName | AndroidIntentRedirectionTest.java:137:40:137:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:136:54:136:59 | intent : Intent | AndroidIntentRedirectionTest.java:136:54:136:89 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:136:54:136:89 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:136:25:136:94 | createRelative(...) : ComponentName | provenance | MaD:240 | +| AndroidIntentRedirectionTest.java:137:17:137:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:138:31:138:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:137:40:137:48 | component : ComponentName | AndroidIntentRedirectionTest.java:137:17:137:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:142:43:143:59 | createRelative(...) : ComponentName | AndroidIntentRedirectionTest.java:144:40:144:48 | component : ComponentName | provenance | | +| AndroidIntentRedirectionTest.java:143:25:143:30 | intent : Intent | AndroidIntentRedirectionTest.java:143:25:143:58 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:143:25:143:58 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:142:43:143:59 | createRelative(...) : ComponentName | provenance | MaD:239 | +| AndroidIntentRedirectionTest.java:144:17:144:25 | fwdIntent [post update] : Intent | AndroidIntentRedirectionTest.java:145:31:145:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:144:40:144:48 | component : ComponentName | AndroidIntentRedirectionTest.java:144:17:144:25 | fwdIntent [post update] : Intent | provenance | MaD:366 | +| AndroidIntentRedirectionTest.java:161:41:161:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:162:45:162:58 | originalIntent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:162:36:162:95 | (...)... : Intent | AndroidIntentRedirectionTest.java:164:35:164:43 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:162:45:162:58 | originalIntent : Intent | AndroidIntentRedirectionTest.java:162:45:162:95 | getParcelableExtra(...) : Parcelable | provenance | MaD:326 | +| AndroidIntentRedirectionTest.java:162:45:162:95 | getParcelableExtra(...) : Parcelable | AndroidIntentRedirectionTest.java:162:36:162:95 | (...)... : Intent | provenance | | +| AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:171:45:171:58 | originalIntent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:172:25:172:38 | originalIntent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:173:31:173:44 | originalIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | AndroidIntentRedirectionTest.java:171:45:171:58 | originalIntent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | AndroidIntentRedirectionTest.java:172:25:172:38 | originalIntent : Intent | provenance | | +| AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | AndroidIntentRedirectionTest.java:173:31:173:44 | originalIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:171:45:171:58 | originalIntent : Intent | AndroidIntentRedirectionTest.java:171:45:171:89 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:171:45:171:89 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | provenance | MaD:364 | +| AndroidIntentRedirectionTest.java:172:25:172:38 | originalIntent : Intent | AndroidIntentRedirectionTest.java:172:25:172:67 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:172:25:172:67 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | provenance | MaD:364 | +| AndroidIntentRedirectionTest.java:192:36:192:88 | parseUri(...) : Intent | AndroidIntentRedirectionTest.java:193:31:193:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:192:52:192:62 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:192:52:192:84 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:192:52:192:84 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:192:36:192:88 | parseUri(...) : Intent | provenance | MaD:332 | +| AndroidIntentRedirectionTest.java:196:36:196:86 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:197:31:197:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:196:53:196:63 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:196:53:196:85 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:196:53:196:85 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:196:36:196:86 | getIntent(...) : Intent | provenance | MaD:321 | +| AndroidIntentRedirectionTest.java:200:36:200:89 | getIntentOld(...) : Intent | AndroidIntentRedirectionTest.java:201:31:201:39 | fwdIntent | provenance | Sink:MaD:227 | +| AndroidIntentRedirectionTest.java:200:56:200:66 | getIntent(...) : Intent | AndroidIntentRedirectionTest.java:200:56:200:88 | getStringExtra(...) : String | provenance | MaD:330 | +| AndroidIntentRedirectionTest.java:200:56:200:88 | getStringExtra(...) : String | AndroidIntentRedirectionTest.java:200:36:200:89 | getIntentOld(...) : Intent | provenance | MaD:323 | +nodes +| AndroidIntentRedirectionTest.java:12:25:12:81 | (...)... : Intent | semmle.label | (...)... : Intent | +| AndroidIntentRedirectionTest.java:12:34:12:44 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:12:34:12:81 | getParcelableExtra(...) : Parcelable | semmle.label | getParcelableExtra(...) : Parcelable | +| AndroidIntentRedirectionTest.java:15:25:15:45 | new Intent[] | semmle.label | new Intent[] | +| AndroidIntentRedirectionTest.java:15:25:15:45 | {...} : Intent[] [[]] : Intent | semmle.label | {...} : Intent[] [[]] : Intent | +| AndroidIntentRedirectionTest.java:15:39:15:44 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:16:25:16:45 | new Intent[] | semmle.label | new Intent[] | +| AndroidIntentRedirectionTest.java:16:25:16:45 | {...} : Intent[] [[]] : Intent | semmle.label | {...} : Intent[] [[]] : Intent | +| AndroidIntentRedirectionTest.java:16:39:16:44 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:17:23:17:28 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:18:23:18:28 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:19:29:19:34 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:20:31:20:36 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:21:32:21:37 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:22:32:22:37 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:23:38:23:43 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:24:38:24:43 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:25:38:25:43 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:26:38:26:43 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:29:22:29:27 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:30:28:30:33 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:31:32:31:37 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:32:23:32:28 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:33:23:33:28 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:34:29:34:34 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:35:29:35:34 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:36:46:36:51 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:37:29:37:34 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:38:35:38:40 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:39:36:39:41 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:40:42:40:47 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:47:27:47:32 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:49:27:49:32 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:52:27:52:32 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:54:27:54:32 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:61:27:61:32 | intent | semmle.label | intent | +| AndroidIntentRedirectionTest.java:67:30:67:40 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:67:30:67:77 | getParcelableExtra(...) : Parcelable | semmle.label | getParcelableExtra(...) : Parcelable | +| AndroidIntentRedirectionTest.java:68:36:68:47 | (...)... : Intent | semmle.label | (...)... : Intent | +| AndroidIntentRedirectionTest.java:69:31:69:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:73:17:73:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:73:56:73:61 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:73:56:73:89 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:74:31:74:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:78:17:78:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:78:40:78:45 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:78:40:78:75 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:79:31:79:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:83:17:83:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:83:40:83:45 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:83:40:83:75 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:84:25:84:30 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:84:25:84:58 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:85:31:85:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:95:17:95:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:95:38:95:43 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:95:38:95:73 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:96:31:96:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:101:25:101:85 | new ComponentName(...) : ComponentName | semmle.label | new ComponentName(...) : ComponentName | +| AndroidIntentRedirectionTest.java:101:43:101:48 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:101:43:101:78 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:102:17:102:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:102:40:102:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:103:31:103:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:107:43:107:99 | new ComponentName(...) : ComponentName | semmle.label | new ComponentName(...) : ComponentName | +| AndroidIntentRedirectionTest.java:107:65:107:70 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:107:65:107:98 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:108:17:108:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:108:40:108:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:109:31:109:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:114:25:114:93 | new ComponentName(...) : ComponentName | semmle.label | new ComponentName(...) : ComponentName | +| AndroidIntentRedirectionTest.java:114:59:114:64 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:114:59:114:92 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:115:17:115:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:115:40:115:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:116:31:116:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:129:25:129:92 | createRelative(...) : ComponentName | semmle.label | createRelative(...) : ComponentName | +| AndroidIntentRedirectionTest.java:129:58:129:63 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:129:58:129:91 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:130:17:130:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:130:40:130:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:131:31:131:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:136:25:136:94 | createRelative(...) : ComponentName | semmle.label | createRelative(...) : ComponentName | +| AndroidIntentRedirectionTest.java:136:54:136:59 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:136:54:136:89 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:137:17:137:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:137:40:137:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:138:31:138:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:142:43:143:59 | createRelative(...) : ComponentName | semmle.label | createRelative(...) : ComponentName | +| AndroidIntentRedirectionTest.java:143:25:143:30 | intent : Intent | semmle.label | intent : Intent | +| AndroidIntentRedirectionTest.java:143:25:143:58 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:144:17:144:25 | fwdIntent [post update] : Intent | semmle.label | fwdIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:144:40:144:48 | component : ComponentName | semmle.label | component : ComponentName | +| AndroidIntentRedirectionTest.java:145:31:145:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:161:41:161:51 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:162:36:162:95 | (...)... : Intent | semmle.label | (...)... : Intent | +| AndroidIntentRedirectionTest.java:162:45:162:58 | originalIntent : Intent | semmle.label | originalIntent : Intent | +| AndroidIntentRedirectionTest.java:162:45:162:95 | getParcelableExtra(...) : Parcelable | semmle.label | getParcelableExtra(...) : Parcelable | +| AndroidIntentRedirectionTest.java:164:35:164:43 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:170:41:170:51 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:171:17:171:30 | originalIntent [post update] : Intent | semmle.label | originalIntent [post update] : Intent | +| AndroidIntentRedirectionTest.java:171:45:171:58 | originalIntent : Intent | semmle.label | originalIntent : Intent | +| AndroidIntentRedirectionTest.java:171:45:171:89 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:172:25:172:38 | originalIntent : Intent | semmle.label | originalIntent : Intent | +| AndroidIntentRedirectionTest.java:172:25:172:67 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:173:31:173:44 | originalIntent | semmle.label | originalIntent | +| AndroidIntentRedirectionTest.java:192:36:192:88 | parseUri(...) : Intent | semmle.label | parseUri(...) : Intent | +| AndroidIntentRedirectionTest.java:192:52:192:62 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:192:52:192:84 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:193:31:193:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:196:36:196:86 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:196:53:196:63 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:196:53:196:85 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:197:31:197:39 | fwdIntent | semmle.label | fwdIntent | +| AndroidIntentRedirectionTest.java:200:36:200:89 | getIntentOld(...) : Intent | semmle.label | getIntentOld(...) : Intent | +| AndroidIntentRedirectionTest.java:200:56:200:66 | getIntent(...) : Intent | semmle.label | getIntent(...) : Intent | +| AndroidIntentRedirectionTest.java:200:56:200:88 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String | +| AndroidIntentRedirectionTest.java:201:31:201:39 | fwdIntent | semmle.label | fwdIntent | +subpaths diff --git a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.java b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.java index 2ce945461b6b..c9d40977c8a2 100644 --- a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.java +++ b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.java @@ -9,80 +9,80 @@ public class AndroidIntentRedirectionTest extends Activity { public void onCreate(Bundle savedInstanceState) { - Intent intent = (Intent) getIntent().getParcelableExtra("forward_intent"); + Intent intent = (Intent) getIntent().getParcelableExtra("forward_intent"); // $ Source=intent // @formatter:off - startActivities(new Intent[] {intent}); // $ hasAndroidIntentRedirection - startActivities(new Intent[] {intent}, null); // $ hasAndroidIntentRedirection - startActivity(intent); // $ hasAndroidIntentRedirection - startActivity(intent, null); // $ hasAndroidIntentRedirection - startActivityAsUser(intent, null); // $ hasAndroidIntentRedirection - startActivityAsCaller(intent, null, false, 0); // $ hasAndroidIntentRedirection - startActivityForResult(intent, 0); // $ hasAndroidIntentRedirection - startActivityForResult(intent, 0, null); // $ hasAndroidIntentRedirection - startActivityForResult(null, intent, 0, null); // $ hasAndroidIntentRedirection - startActivityForResultAsUser(intent, null, 0, null, null); // $ hasAndroidIntentRedirection - startActivityForResultAsUser(intent, 0, null, null); // $ hasAndroidIntentRedirection - startActivityForResultAsUser(intent, 0, null); // $ hasAndroidIntentRedirection + startActivities(new Intent[] {intent}); // $ Alert=intent + startActivities(new Intent[] {intent}, null); // $ Alert=intent + startActivity(intent); // $ Alert=intent + startActivity(intent, null); // $ Alert=intent + startActivityAsUser(intent, null); // $ Alert=intent + startActivityAsCaller(intent, null, false, 0); // $ Alert=intent + startActivityForResult(intent, 0); // $ Alert=intent + startActivityForResult(intent, 0, null); // $ Alert=intent + startActivityForResult(null, intent, 0, null); // $ Alert=intent + startActivityForResultAsUser(intent, null, 0, null, null); // $ Alert=intent + startActivityForResultAsUser(intent, 0, null, null); // $ Alert=intent + startActivityForResultAsUser(intent, 0, null); // $ Alert=intent bindService(intent, null, 0); bindServiceAsUser(intent, null, 0, null); - startService(intent); // $ hasAndroidIntentRedirection - startServiceAsUser(intent, null); // $ hasAndroidIntentRedirection - startForegroundService(intent); // $ hasAndroidIntentRedirection - sendBroadcast(intent); // $ hasAndroidIntentRedirection - sendBroadcast(intent, null); // $ hasAndroidIntentRedirection - sendBroadcastAsUser(intent, null); // $ hasAndroidIntentRedirection - sendBroadcastAsUser(intent, null, null); // $ hasAndroidIntentRedirection - sendBroadcastWithMultiplePermissions(intent, null); // $ hasAndroidIntentRedirection - sendStickyBroadcast(intent); // $ hasAndroidIntentRedirection - sendStickyBroadcastAsUser(intent, null); // $ hasAndroidIntentRedirection - sendStickyOrderedBroadcast(intent, null, null, 0, null, null); // $ hasAndroidIntentRedirection - sendStickyOrderedBroadcastAsUser(intent, null, null, null, 0, null, null); // $ hasAndroidIntentRedirection + startService(intent); // $ Alert=intent + startServiceAsUser(intent, null); // $ Alert=intent + startForegroundService(intent); // $ Alert=intent + sendBroadcast(intent); // $ Alert=intent + sendBroadcast(intent, null); // $ Alert=intent + sendBroadcastAsUser(intent, null); // $ Alert=intent + sendBroadcastAsUser(intent, null, null); // $ Alert=intent + sendBroadcastWithMultiplePermissions(intent, null); // $ Alert=intent + sendStickyBroadcast(intent); // $ Alert=intent + sendStickyBroadcastAsUser(intent, null); // $ Alert=intent + sendStickyOrderedBroadcast(intent, null, null, 0, null, null); // $ Alert=intent + sendStickyOrderedBroadcastAsUser(intent, null, null, null, 0, null, null); // $ Alert=intent // @formatter:on // Sanitizing only the package or the class still allows redirecting // to non-exported activities in the same package // or activities with the same name in other packages, respectively. if (intent.getComponent().getPackageName().equals("something")) { - startActivity(intent); // $ hasAndroidIntentRedirection + startActivity(intent); // $ Alert=intent } else { - startActivity(intent); // $ hasAndroidIntentRedirection + startActivity(intent); // $ Alert=intent } if (intent.getComponent().getClassName().equals("something")) { - startActivity(intent); // $ hasAndroidIntentRedirection + startActivity(intent); // $ Alert=intent } else { - startActivity(intent); // $ hasAndroidIntentRedirection + startActivity(intent); // $ Alert=intent } if (intent.getComponent().getPackageName().equals("something") && intent.getComponent().getClassName().equals("something")) { startActivity(intent); // Safe } else { - startActivity(intent); // $ hasAndroidIntentRedirection + startActivity(intent); // $ Alert=intent } try { { // Delayed cast - Object obj = getIntent().getParcelableExtra("forward_intent"); + Object obj = getIntent().getParcelableExtra("forward_intent"); // $ Source=intent2 Intent fwdIntent = (Intent) obj; - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent2 } { Intent fwdIntent = new Intent(); fwdIntent.setClassName((Context) null, intent.getStringExtra("className")); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); fwdIntent.setClassName(intent.getStringExtra("packageName"), null); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); fwdIntent.setClassName(intent.getStringExtra("packageName"), intent.getStringExtra("className")); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); @@ -93,27 +93,27 @@ public void onCreate(Bundle savedInstanceState) { { Intent fwdIntent = new Intent(); fwdIntent.setPackage(intent.getStringExtra("packageName")); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); ComponentName component = new ComponentName(intent.getStringExtra("packageName"), null); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); ComponentName component = new ComponentName("", intent.getStringExtra("className")); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); ComponentName component = new ComponentName((Context) null, intent.getStringExtra("className")); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); @@ -128,21 +128,21 @@ public void onCreate(Bundle savedInstanceState) { ComponentName component = ComponentName.createRelative("", intent.getStringExtra("className")); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); ComponentName component = ComponentName.createRelative(intent.getStringExtra("packageName"), ""); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent fwdIntent = new Intent(); ComponentName component = ComponentName.createRelative((Context) null, intent.getStringExtra("className")); fwdIntent.setComponent(component); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent } { Intent originalIntent = getIntent(); @@ -158,19 +158,19 @@ public void onCreate(Bundle savedInstanceState) { startActivity(anotherIntent); // Safe - copy constructor from original Intent } { - Intent originalIntent = getIntent(); + Intent originalIntent = getIntent(); // $ Source=intent3 Intent fwdIntent = (Intent) originalIntent.getParcelableExtra("forward_intent"); if (originalIntent.getBooleanExtra("use_fwd_intent", false)) { - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + startActivity(fwdIntent); // $ Alert=intent3 } else { startActivity(originalIntent); // Safe - not an Intent obtained from the Extras } } { - Intent originalIntent = getIntent(); + Intent originalIntent = getIntent(); // $ Source=intent4 originalIntent.setClassName(originalIntent.getStringExtra("package_name"), originalIntent.getStringExtra("class_name")); - startActivity(originalIntent); // $ hasAndroidIntentRedirection + startActivity(originalIntent); // $ Alert=intent4 } { Intent originalIntent = getIntent(); @@ -189,16 +189,16 @@ public void onCreate(Bundle savedInstanceState) { startActivity(fwdIntent); // $ MISSING: $hasAndroidIntentRedirection } { - Intent fwdIntent = Intent.parseUri(getIntent().getStringExtra("uri"), 0); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + Intent fwdIntent = Intent.parseUri(getIntent().getStringExtra("uri"), 0); // $ Source=intent5 + startActivity(fwdIntent); // $ Alert=intent5 } { - Intent fwdIntent = Intent.getIntent(getIntent().getStringExtra("uri")); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + Intent fwdIntent = Intent.getIntent(getIntent().getStringExtra("uri")); // $ Source=intent6 + startActivity(fwdIntent); // $ Alert=intent6 } { - Intent fwdIntent = Intent.getIntentOld(getIntent().getStringExtra("uri")); - startActivity(fwdIntent); // $ hasAndroidIntentRedirection + Intent fwdIntent = Intent.getIntentOld(getIntent().getStringExtra("uri")); // $ Source=intent7 + startActivity(fwdIntent); // $ Alert=intent7 } } catch (Exception e) { } diff --git a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.ql b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.ql deleted file mode 100644 index 6c4d121a2bcf..000000000000 --- a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.ql +++ /dev/null @@ -1,18 +0,0 @@ -import java -import semmle.code.java.security.AndroidIntentRedirectionQuery -import TestUtilities.InlineExpectationsTest - -module HasAndroidIntentRedirectionTest implements TestSig { - string getARelevantTag() { result = "hasAndroidIntentRedirection" } - - predicate hasActualResult(Location location, string element, string tag, string value) { - tag = "hasAndroidIntentRedirection" and - exists(DataFlow::Node sink | IntentRedirectionFlow::flowTo(sink) | - sink.getLocation() = location and - element = sink.toString() and - value = "" - ) - } -} - -import MakeTest diff --git a/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.qlref b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.qlref new file mode 100644 index 000000000000..e6061ac902a2 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-940/AndroidIntentRedirectionTest.qlref @@ -0,0 +1,2 @@ +query: Security/CWE/CWE-940/AndroidIntentRedirection.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file From 5b5ca05e878828d649c32e3416de7037c56defde Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 09:47:56 +0200 Subject: [PATCH 05/15] Ruby: Post-processing query for inline test expectations --- .../InlineExpectationsTestQuery.ql | 21 +++++++ .../cwe-022/ArchiveApiPathTraversal.rb | 12 ++-- .../security/cwe-022/PathInjection.expected | 38 ++++++------- .../security/cwe-022/PathInjection.qlref | 3 +- .../security/cwe-022/tainted_path.rb | 56 +++++++++---------- 5 files changed, 76 insertions(+), 54 deletions(-) create mode 100644 ruby/ql/test/TestUtilities/InlineExpectationsTestQuery.ql diff --git a/ruby/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/ruby/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..1cbc37a7fe85 --- /dev/null +++ b/ruby/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import ruby +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/ruby/ql/test/query-tests/security/cwe-022/ArchiveApiPathTraversal.rb b/ruby/ql/test/query-tests/security/cwe-022/ArchiveApiPathTraversal.rb index 9708d3770bcb..046322fdbc83 100644 --- a/ruby/ql/test/query-tests/security/cwe-022/ArchiveApiPathTraversal.rb +++ b/ruby/ql/test/query-tests/security/cwe-022/ArchiveApiPathTraversal.rb @@ -2,17 +2,17 @@ class TestContoller < ActionController::Base # this is vulnerable def upload - untar params[:file], params[:filename] + untar params[:file], params[:filename] # $ Source=upload end # this is vulnerable def unpload_zip - unzip params[:file] + unzip params[:file] # $ Source=upload_zip end # this is vulnerable def create_new_zip - zip params[:filename], files + zip params[:filename], files # $ Source=create_new_zip end # these are not vulnerable because of the string compare sanitizer @@ -56,7 +56,7 @@ def untar(io, destination) else destination_directory = File.dirname(destination_file) FileUtils.mkdir_p destination_directory unless File.directory?(destination_directory) - File.open destination_file, "wb" do |f| + File.open destination_file, "wb" do |f| # $ Alert=upload f.print tarfile.read end end @@ -65,7 +65,7 @@ def untar(io, destination) end def unzip(file) - Zip::File.open(file) do |zip_file| + Zip::File.open(file) do |zip_file| # $ Alert=upload_zip zip_file.each do |entry| entry.extract end @@ -73,7 +73,7 @@ def unzip(file) end def zip(filename, files = []) - Zip::File.new(filename) do |zf| + Zip::File.new(filename) do |zf| # $ Alert=create_new_zip files.each do |f| zf.add f end diff --git a/ruby/ql/test/query-tests/security/cwe-022/PathInjection.expected b/ruby/ql/test/query-tests/security/cwe-022/PathInjection.expected index cf65d245723d..af3a962886b7 100644 --- a/ruby/ql/test/query-tests/security/cwe-022/PathInjection.expected +++ b/ruby/ql/test/query-tests/security/cwe-022/PathInjection.expected @@ -1,3 +1,22 @@ +#select +| ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | This path depends on a $@. | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | user-provided value | +| ArchiveApiPathTraversal.rb:68:20:68:23 | file | ArchiveApiPathTraversal.rb:10:11:10:16 | call to params | ArchiveApiPathTraversal.rb:68:20:68:23 | file | This path depends on a $@. | ArchiveApiPathTraversal.rb:10:11:10:16 | call to params | user-provided value | +| ArchiveApiPathTraversal.rb:76:19:76:26 | filename | ArchiveApiPathTraversal.rb:15:9:15:14 | call to params | ArchiveApiPathTraversal.rb:76:19:76:26 | filename | This path depends on a $@. | ArchiveApiPathTraversal.rb:15:9:15:14 | call to params | user-provided value | +| tainted_path.rb:5:26:5:29 | path | tainted_path.rb:4:12:4:17 | call to params | tainted_path.rb:5:26:5:29 | path | This path depends on a $@. | tainted_path.rb:4:12:4:17 | call to params | user-provided value | +| tainted_path.rb:11:26:11:29 | path | tainted_path.rb:10:31:10:36 | call to params | tainted_path.rb:11:26:11:29 | path | This path depends on a $@. | tainted_path.rb:10:31:10:36 | call to params | user-provided value | +| tainted_path.rb:17:26:17:29 | path | tainted_path.rb:16:28:16:33 | call to params | tainted_path.rb:17:26:17:29 | path | This path depends on a $@. | tainted_path.rb:16:28:16:33 | call to params | user-provided value | +| tainted_path.rb:23:26:23:29 | path | tainted_path.rb:22:29:22:34 | call to params | tainted_path.rb:23:26:23:29 | path | This path depends on a $@. | tainted_path.rb:22:29:22:34 | call to params | user-provided value | +| tainted_path.rb:29:26:29:29 | path | tainted_path.rb:28:22:28:27 | call to params | tainted_path.rb:29:26:29:29 | path | This path depends on a $@. | tainted_path.rb:28:22:28:27 | call to params | user-provided value | +| tainted_path.rb:35:26:35:29 | path | tainted_path.rb:34:29:34:34 | call to params | tainted_path.rb:35:26:35:29 | path | This path depends on a $@. | tainted_path.rb:34:29:34:34 | call to params | user-provided value | +| tainted_path.rb:41:26:41:29 | path | tainted_path.rb:40:26:40:31 | call to params | tainted_path.rb:41:26:41:29 | path | This path depends on a $@. | tainted_path.rb:40:26:40:31 | call to params | user-provided value | +| tainted_path.rb:48:26:48:29 | path | tainted_path.rb:47:43:47:48 | call to params | tainted_path.rb:48:26:48:29 | path | This path depends on a $@. | tainted_path.rb:47:43:47:48 | call to params | user-provided value | +| tainted_path.rb:60:26:60:29 | path | tainted_path.rb:59:40:59:45 | call to params | tainted_path.rb:60:26:60:29 | path | This path depends on a $@. | tainted_path.rb:59:40:59:45 | call to params | user-provided value | +| tainted_path.rb:72:15:72:18 | path | tainted_path.rb:71:40:71:45 | call to params | tainted_path.rb:72:15:72:18 | path | This path depends on a $@. | tainted_path.rb:71:40:71:45 | call to params | user-provided value | +| tainted_path.rb:78:19:78:22 | path | tainted_path.rb:77:40:77:45 | call to params | tainted_path.rb:78:19:78:22 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value | +| tainted_path.rb:79:14:79:17 | path | tainted_path.rb:77:40:77:45 | call to params | tainted_path.rb:79:14:79:17 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value | +| tainted_path.rb:85:10:85:13 | path | tainted_path.rb:84:40:84:45 | call to params | tainted_path.rb:85:10:85:13 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value | +| tainted_path.rb:86:25:86:28 | path | tainted_path.rb:84:40:84:45 | call to params | tainted_path.rb:86:25:86:28 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value | +| tainted_path.rb:92:11:92:14 | path | tainted_path.rb:90:40:90:45 | call to params | tainted_path.rb:92:11:92:14 | path | This path depends on a $@. | tainted_path.rb:90:40:90:45 | call to params | user-provided value | edges | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | ArchiveApiPathTraversal.rb:5:26:5:42 | ...[...] | provenance | | | ArchiveApiPathTraversal.rb:5:26:5:42 | ...[...] | ArchiveApiPathTraversal.rb:49:17:49:27 | destination | provenance | | @@ -152,22 +171,3 @@ nodes | tainted_path.rb:90:40:90:52 | ...[...] | semmle.label | ...[...] | | tainted_path.rb:92:11:92:14 | path | semmle.label | path | subpaths -#select -| ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | This path depends on a $@. | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | user-provided value | -| ArchiveApiPathTraversal.rb:68:20:68:23 | file | ArchiveApiPathTraversal.rb:10:11:10:16 | call to params | ArchiveApiPathTraversal.rb:68:20:68:23 | file | This path depends on a $@. | ArchiveApiPathTraversal.rb:10:11:10:16 | call to params | user-provided value | -| ArchiveApiPathTraversal.rb:76:19:76:26 | filename | ArchiveApiPathTraversal.rb:15:9:15:14 | call to params | ArchiveApiPathTraversal.rb:76:19:76:26 | filename | This path depends on a $@. | ArchiveApiPathTraversal.rb:15:9:15:14 | call to params | user-provided value | -| tainted_path.rb:5:26:5:29 | path | tainted_path.rb:4:12:4:17 | call to params | tainted_path.rb:5:26:5:29 | path | This path depends on a $@. | tainted_path.rb:4:12:4:17 | call to params | user-provided value | -| tainted_path.rb:11:26:11:29 | path | tainted_path.rb:10:31:10:36 | call to params | tainted_path.rb:11:26:11:29 | path | This path depends on a $@. | tainted_path.rb:10:31:10:36 | call to params | user-provided value | -| tainted_path.rb:17:26:17:29 | path | tainted_path.rb:16:28:16:33 | call to params | tainted_path.rb:17:26:17:29 | path | This path depends on a $@. | tainted_path.rb:16:28:16:33 | call to params | user-provided value | -| tainted_path.rb:23:26:23:29 | path | tainted_path.rb:22:29:22:34 | call to params | tainted_path.rb:23:26:23:29 | path | This path depends on a $@. | tainted_path.rb:22:29:22:34 | call to params | user-provided value | -| tainted_path.rb:29:26:29:29 | path | tainted_path.rb:28:22:28:27 | call to params | tainted_path.rb:29:26:29:29 | path | This path depends on a $@. | tainted_path.rb:28:22:28:27 | call to params | user-provided value | -| tainted_path.rb:35:26:35:29 | path | tainted_path.rb:34:29:34:34 | call to params | tainted_path.rb:35:26:35:29 | path | This path depends on a $@. | tainted_path.rb:34:29:34:34 | call to params | user-provided value | -| tainted_path.rb:41:26:41:29 | path | tainted_path.rb:40:26:40:31 | call to params | tainted_path.rb:41:26:41:29 | path | This path depends on a $@. | tainted_path.rb:40:26:40:31 | call to params | user-provided value | -| tainted_path.rb:48:26:48:29 | path | tainted_path.rb:47:43:47:48 | call to params | tainted_path.rb:48:26:48:29 | path | This path depends on a $@. | tainted_path.rb:47:43:47:48 | call to params | user-provided value | -| tainted_path.rb:60:26:60:29 | path | tainted_path.rb:59:40:59:45 | call to params | tainted_path.rb:60:26:60:29 | path | This path depends on a $@. | tainted_path.rb:59:40:59:45 | call to params | user-provided value | -| tainted_path.rb:72:15:72:18 | path | tainted_path.rb:71:40:71:45 | call to params | tainted_path.rb:72:15:72:18 | path | This path depends on a $@. | tainted_path.rb:71:40:71:45 | call to params | user-provided value | -| tainted_path.rb:78:19:78:22 | path | tainted_path.rb:77:40:77:45 | call to params | tainted_path.rb:78:19:78:22 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value | -| tainted_path.rb:79:14:79:17 | path | tainted_path.rb:77:40:77:45 | call to params | tainted_path.rb:79:14:79:17 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value | -| tainted_path.rb:85:10:85:13 | path | tainted_path.rb:84:40:84:45 | call to params | tainted_path.rb:85:10:85:13 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value | -| tainted_path.rb:86:25:86:28 | path | tainted_path.rb:84:40:84:45 | call to params | tainted_path.rb:86:25:86:28 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value | -| tainted_path.rb:92:11:92:14 | path | tainted_path.rb:90:40:90:45 | call to params | tainted_path.rb:92:11:92:14 | path | This path depends on a $@. | tainted_path.rb:90:40:90:45 | call to params | user-provided value | diff --git a/ruby/ql/test/query-tests/security/cwe-022/PathInjection.qlref b/ruby/ql/test/query-tests/security/cwe-022/PathInjection.qlref index 7b98278f7a78..07015c904352 100644 --- a/ruby/ql/test/query-tests/security/cwe-022/PathInjection.qlref +++ b/ruby/ql/test/query-tests/security/cwe-022/PathInjection.qlref @@ -1 +1,2 @@ -queries/security/cwe-022/PathInjection.ql +query: queries/security/cwe-022/PathInjection.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql diff --git a/ruby/ql/test/query-tests/security/cwe-022/tainted_path.rb b/ruby/ql/test/query-tests/security/cwe-022/tainted_path.rb index d47607e67342..b7e89442c523 100644 --- a/ruby/ql/test/query-tests/security/cwe-022/tainted_path.rb +++ b/ruby/ql/test/query-tests/security/cwe-022/tainted_path.rb @@ -1,51 +1,51 @@ class FooController < ActionController::Base # BAD def route0 - path = params[:path] - @content = File.read path + path = params[:path] # $ Source=path1 + @content = File.read path # $ Alert=path1 end # BAD - File.absolute_path preserves taint def route1 - path = File.absolute_path params[:path] - @content = File.read path + path = File.absolute_path params[:path] # $ Source=path2 + @content = File.read path # $ Alert=path2 end # BAD - File.dirname preserves taint def route2 - path = "#{File.dirname(params[:path])}/foo" - @content = File.read path + path = "#{File.dirname(params[:path])}/foo" # $ Source=path3 + @content = File.read path # $ Alert=path3 end # BAD - File.expand_path preserves taint def route3 - path = File.expand_path params[:path] - @content = File.read path + path = File.expand_path params[:path] # $ Source=path4 + @content = File.read path # $ Alert=path4 end # BAD - File.path preserves taint def route4 - path = File.path params[:path] - @content = File.read path + path = File.path params[:path] # $ Source=path5 + @content = File.read path # $ Alert=path5 end # BAD - File.realdirpath preserves taint def route5 - path = File.realdirpath params[:path] - @content = File.read path + path = File.realdirpath params[:path] # $ Source=path6 + @content = File.read path # $ Alert=path6 end # BAD - File.realpath preserves taint def route6 - path = File.realpath params[:path] - @content = File.read path + path = File.realpath params[:path] # $ Source=path7 + @content = File.read path # $ Alert=path7 end # BAD - tainted arguments in any position propagate to the return value of # File.join def route7 - path = File.join("foo", "bar", "baz", params[:path], "qux") - @content = File.read path + path = File.join("foo", "bar", "baz", params[:path], "qux") # $ Source=path8 + @content = File.read path # $ Alert=path8 end # GOOD - File.basename does not preserve taint @@ -56,8 +56,8 @@ def route8 # BAD def route9 - path = ActiveStorage::Filename.new(params[:path]) - @content = File.read path + path = ActiveStorage::Filename.new(params[:path]) # $ Source=path9 + @content = File.read path # $ Alert=path9 end # GOOD - explicitly sanitized @@ -68,27 +68,27 @@ def route10 # BAD def route11 - path = ActiveStorage::Filename.new(params[:path]) - send_file path + path = ActiveStorage::Filename.new(params[:path]) # $ Source=path10 + send_file path # $ Alert=path10 end # BAD def route12 - path = ActiveStorage::Filename.new(params[:path]) - bla (Dir.glob path) - bla (Dir[path]) + path = ActiveStorage::Filename.new(params[:path]) # $ Source=path11 + bla (Dir.glob path) # $ Alert=path11 + bla (Dir[path]) # $ Alert=path11 end # BAD def route13 - path = ActiveStorage::Filename.new(params[:path]) - load(path) - autoload(:MyModule, path) + path = ActiveStorage::Filename.new(params[:path]) # $ Source=path12 + load(path) # $ Alert=path12 + autoload(:MyModule, path) # $ Alert=path12 end def require_relative() - path = ActiveStorage::Filename.new(params[:path]) + path = ActiveStorage::Filename.new(params[:path]) # $ Source=path13 puts "Debug: require_relative(#{path})" - super(path) + super(path) # $ Alert=path13 end end From 4561770db4ce2a552e7fd58f02d36e609272afba Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 10:10:40 +0200 Subject: [PATCH 06/15] Swift: Post-processing query for inline test expectations --- .../TestUtilities/InlineExpectationsTest.qll | 29 +---------- .../InlineExpectationsTestQuery.ql | 21 ++++++++ .../internal/InlineExpectationsTestImpl.qll | 28 +++++++++++ .../Security/CWE-094/UnsafeJsEval.qlref | 3 +- .../Security/CWE-094/UnsafeJsEval.swift | 50 +++++++++---------- 5 files changed, 77 insertions(+), 54 deletions(-) create mode 100644 swift/ql/test/TestUtilities/InlineExpectationsTestQuery.ql create mode 100644 swift/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll diff --git a/swift/ql/test/TestUtilities/InlineExpectationsTest.qll b/swift/ql/test/TestUtilities/InlineExpectationsTest.qll index 96c537a022b6..da8cde37b93f 100644 --- a/swift/ql/test/TestUtilities/InlineExpectationsTest.qll +++ b/swift/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -3,33 +3,6 @@ * See `shared/util/codeql/util/test/InlineExpectationsTest.qll` */ -private import swift as S private import codeql.util.test.InlineExpectationsTest - -private module Impl implements InlineExpectationsTestSig { - private newtype TExpectationComment = MkExpectationComment(S::SingleLineComment c) - - /** - * A class representing a line comment. - * Unlike the `SingleLineComment` class, however, the string returned by `getContents` does _not_ - * include the preceding comment marker (`//`). - */ - class ExpectationComment extends TExpectationComment { - S::SingleLineComment comment; - - ExpectationComment() { this = MkExpectationComment(comment) } - - /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */ - string getContents() { result = comment.getText().suffix(2) } - - /** Gets a textual representation of this element. */ - string toString() { result = comment.toString() } - - /** Gets the location of this comment. */ - Location getLocation() { result = comment.getLocation() } - } - - class Location = S::Location; -} - +private import internal.InlineExpectationsTestImpl import Make diff --git a/swift/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/swift/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..a7c112bc00e0 --- /dev/null +++ b/swift/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import swift +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/swift/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll b/swift/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll new file mode 100644 index 000000000000..af84a908633b --- /dev/null +++ b/swift/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll @@ -0,0 +1,28 @@ +private import swift as S +private import codeql.util.test.InlineExpectationsTest + +module Impl implements InlineExpectationsTestSig { + private newtype TExpectationComment = MkExpectationComment(S::SingleLineComment c) + + /** + * A class representing a line comment. + * Unlike the `SingleLineComment` class, however, the string returned by `getContents` does _not_ + * include the preceding comment marker (`//`). + */ + class ExpectationComment extends TExpectationComment { + S::SingleLineComment comment; + + ExpectationComment() { this = MkExpectationComment(comment) } + + /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */ + string getContents() { result = comment.getText().suffix(2) } + + /** Gets a textual representation of this element. */ + string toString() { result = comment.toString() } + + /** Gets the location of this comment. */ + Location getLocation() { result = comment.getLocation() } + } + + class Location = S::Location; +} diff --git a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.qlref b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.qlref index b8c11cee30df..51ad8bf6ed37 100644 --- a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.qlref +++ b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.qlref @@ -1 +1,2 @@ -queries/Security/CWE-094/UnsafeJsEval.ql +query: queries/Security/CWE-094/UnsafeJsEval.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql diff --git a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift index 83d691d5f031..a0b9fd0d78ce 100644 --- a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift +++ b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift @@ -175,17 +175,17 @@ func testAsync(_ sink: @escaping (String) async throws -> ()) { let url = URL(string: "http://example.com/") try! await sink(localString) // GOOD: the HTML data is local - try! await sink(try String(contentsOf: URL(string: "http://example.com/")!)) // BAD [NOT DETECTED - TODO]: HTML contains remote input, may access local secrets - try! await sink(try! String(contentsOf: url!)) // BAD [NOT DETECTED - TODO] + try! await sink(try String(contentsOf: URL(string: "http://example.com/")!)) // $ MISSING: Alert (HTML contains remote input, may access local secrets) + try! await sink(try! String(contentsOf: url!)) // $ MISSING: Alert try! await sink("console.log(" + localStringFragment + ")") // GOOD: the HTML data is local - try! await sink("console.log(" + (try! String(contentsOf: url!)) + ")") // BAD [NOT DETECTED - TODO] + try! await sink("console.log(" + (try! String(contentsOf: url!)) + ")") // $ MISSING: Alert let localData = Data(localString.utf8) let remoteData = Data((try! String(contentsOf: url!)).utf8) try! await sink(String(decoding: localData, as: UTF8.self)) // GOOD: the data is local - try! await sink(String(decoding: remoteData, as: UTF8.self)) // BAD [NOT DETECTED - TODO]: the data is remote + try! await sink(String(decoding: remoteData, as: UTF8.self)) // $ MISSING: Alert the data is remote try! await sink("console.log(" + String(Int(localStringFragment) ?? 0) + ")") // GOOD: Primitive conversion try! await sink("console.log(" + String(Int(try! String(contentsOf: url!)) ?? 0) + ")") // GOOD: Primitive conversion @@ -201,17 +201,17 @@ func testSync(_ sink: @escaping (String) -> ()) { let url = URL(string: "http://example.com/") sink(localString) // GOOD: the HTML data is local - sink(try! String(contentsOf: URL(string: "http://example.com/")!)) // BAD: HTML contains remote input, may access local secrets - sink(try! String(contentsOf: url!)) // BAD + sink(try! String(contentsOf: URL(string: "http://example.com/")!)) // $ Source=source1 $ MISSING: Alert HTML contains remote input, may access local secrets + sink(try! String(contentsOf: url!)) // $ Source=source2 $ MISSING: Alert sink("console.log(" + localStringFragment + ")") // GOOD: the HTML data is local - sink("console.log(" + (try! String(contentsOf: url!)) + ")") // BAD + sink("console.log(" + (try! String(contentsOf: url!)) + ")") // $ Source=source3 $ MISSING: Alert let localData = Data(localString.utf8) - let remoteData = Data((try! String(contentsOf: url!)).utf8) + let remoteData = Data((try! String(contentsOf: url!)).utf8) // $ Source=source4 sink(String(decoding: localData, as: UTF8.self)) // GOOD: the data is local - sink(String(decoding: remoteData, as: UTF8.self)) // BAD: the data is remote + sink(String(decoding: remoteData, as: UTF8.self)) // $ MISSING: Alert the data is remote sink("console.log(" + String(Int(localStringFragment) ?? 0) + ")") // GOOD: Primitive conversion sink("console.log(" + String(Int(try! String(contentsOf: url!)) ?? 0) + ")") // GOOD: Primitive conversion @@ -224,7 +224,7 @@ func testUIWebView() { let webview = UIWebView() testAsync { string in - _ = await webview.stringByEvaluatingJavaScript(from: string) // BAD [NOT DETECTED] + _ = await webview.stringByEvaluatingJavaScript(from: string) // $ MISSING: Alert } } @@ -232,7 +232,7 @@ func testWebView() { let webview = WebView() testAsync { string in - _ = await webview.stringByEvaluatingJavaScript(from: string) // BAD [NOT DETECTED] + _ = await webview.stringByEvaluatingJavaScript(from: string) // $ MISSING: Alert } } @@ -240,22 +240,22 @@ func testWKWebView() { let webview = WKWebView() testAsync { string in - _ = try await webview.evaluateJavaScript(string) // BAD [NOT DETECTED] + _ = try await webview.evaluateJavaScript(string) // $ MISSING: Alert } testAsync { string in - await webview.evaluateJavaScript(string) { _, _ in } // BAD [NOT DETECTED] + await webview.evaluateJavaScript(string) { _, _ in } // $ MISSING: Alert } testAsync { string in - await webview.evaluateJavaScript(string, in: nil, in: WKContentWorld.defaultClient) { _ in } // BAD [NOT DETECTED] + await webview.evaluateJavaScript(string, in: nil, in: WKContentWorld.defaultClient) { _ in } // $ MISSING: Alert } testAsync { string in - _ = try await webview.evaluateJavaScript(string, contentWorld: .defaultClient) // BAD [NOT DETECTED] + _ = try await webview.evaluateJavaScript(string, contentWorld: .defaultClient) // $ MISSING: Alert } testAsync { string in - await webview.callAsyncJavaScript(string, in: nil, in: .defaultClient) { _ in () } // BAD [NOT DETECTED] + await webview.callAsyncJavaScript(string, in: nil, in: .defaultClient) { _ in () } // $ MISSING: Alert } testAsync { string in - _ = try await webview.callAsyncJavaScript(string, contentWorld: WKContentWorld.defaultClient) // BAD [NOT DETECTED] + _ = try await webview.callAsyncJavaScript(string, contentWorld: WKContentWorld.defaultClient) // $ MISSING: Alert } } @@ -263,10 +263,10 @@ func testWKUserContentController() { let ctrl = WKUserContentController() testSync { string in - ctrl.addUserScript(WKUserScript(source: string, injectionTime: .atDocumentStart, forMainFrameOnly: false)) // BAD (multiple sources) + ctrl.addUserScript(WKUserScript(source: string, injectionTime: .atDocumentStart, forMainFrameOnly: false)) // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 } testSync { string in - ctrl.addUserScript(WKUserScript(source: string, injectionTime: .atDocumentEnd, forMainFrameOnly: true, in: .defaultClient)) // BAD (multiple sources) + ctrl.addUserScript(WKUserScript(source: string, injectionTime: .atDocumentEnd, forMainFrameOnly: true, in: .defaultClient)) // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 } } @@ -274,10 +274,10 @@ func testJSContext() { let ctx = JSContext() testSync { string in - _ = ctx.evaluateScript(string) // BAD (multiple sources) + _ = ctx.evaluateScript(string) // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 } testSync { string in - _ = ctx.evaluateScript(string, withSourceURL: URL(string: "https://example.com")) // BAD (multiple sources) + _ = ctx.evaluateScript(string, withSourceURL: URL(string: "https://example.com")) // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 } } @@ -288,7 +288,7 @@ func testJSEvaluateScript() { defer { JSStringRelease(jsstr) } _ = JSEvaluateScript( /*ctx:*/ OpaquePointer(bitPattern: 0), - /*script:*/ jsstr, // BAD (multiple sources) + /*script:*/ jsstr, // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 /*thisObject:*/ OpaquePointer(bitPattern: 0), /*sourceURL:*/ OpaquePointer(bitPattern: 0), /*startingLineNumber:*/ 0, @@ -302,7 +302,7 @@ func testJSEvaluateScript() { defer { JSStringRelease(jsstr) } _ = JSEvaluateScript( /*ctx:*/ OpaquePointer(bitPattern: 0), - /*script:*/ jsstr, // BAD (multiple sources) + /*script:*/ jsstr, // $ Alert=source1 $ Alert=source2 $ Alert=source3 $ Alert=source4 /*thisObject:*/ OpaquePointer(bitPattern: 0), /*sourceURL:*/ OpaquePointer(bitPattern: 0), /*startingLineNumber:*/ 0, @@ -315,9 +315,9 @@ func testJSEvaluateScript() { func testQHelpExamples() { Task { let webview = WKWebView() - let remoteData = try String(contentsOf: URL(string: "http://example.com/evil.json")!) + let remoteData = try String(contentsOf: URL(string: "http://example.com/evil.json")!) // $ Source=source5 - _ = try await webview.evaluateJavaScript("console.log(" + remoteData + ")") // BAD + _ = try await webview.evaluateJavaScript("console.log(" + remoteData + ")") // $ Alert=source5 _ = try await webview.callAsyncJavaScript( "console.log(data)", From 540b433f5a5aed17d2e7859aa6aee77b4149b1e3 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 11:48:10 +0200 Subject: [PATCH 07/15] Go: Post-processing query for inline test expectations --- .../InlineExpectationsTestQuery.ql | 21 +++++++++++++++++++ .../semmle/go/frameworks/Gin/Gin.go | 10 ++++----- .../go/frameworks/Gin/TaintedPath.qlref | 4 +++- 3 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 go/ql/test/TestUtilities/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/go/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..1cf2f5ea1d9b --- /dev/null +++ b/go/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import go +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.go b/go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.go index 33e826d5f474..ba8544bf51d6 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.go +++ b/go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.go @@ -21,12 +21,12 @@ type Person struct { func FileSystemAccess() { router := gin.Default() router.POST("/FormUploads", func(c *gin.Context) { - filepath := c.Query("filepath") - c.File(filepath) // $ FileSystemAccess=filepath - http.ServeFile(c.Writer, c.Request, filepath) // $ FileSystemAccess=filepath - c.FileAttachment(filepath, "file name in response") // $ FileSystemAccess=filepath + filepath := c.Query("filepath") // $ Source=filepath + c.File(filepath) // $ Alert=filepath $ FileSystemAccess=filepath + http.ServeFile(c.Writer, c.Request, filepath) // $ Alert=filepath $ FileSystemAccess=filepath + c.FileAttachment(filepath, "file name in response") // $ Alert=filepath $ FileSystemAccess=filepath file, _ := c.FormFile("afile") - _ = c.SaveUploadedFile(file, filepath) // $ FileSystemAccess=filepath + _ = c.SaveUploadedFile(file, filepath) // $ Alert=filepath $ FileSystemAccess=filepath }) _ = router.Run() } diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Gin/TaintedPath.qlref b/go/ql/test/library-tests/semmle/go/frameworks/Gin/TaintedPath.qlref index a90879489725..fffd4b2e8bf5 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Gin/TaintedPath.qlref +++ b/go/ql/test/library-tests/semmle/go/frameworks/Gin/TaintedPath.qlref @@ -1,2 +1,4 @@ query: Security/CWE-022/TaintedPath.ql -postprocess: TestUtilities/PrettyPrintModels.ql +postprocess: + - TestUtilities/PrettyPrintModels.ql + - TestUtilities/InlineExpectationsTestQuery.ql From 4750b0de94be865075c830312730fc4a4942d5f3 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 14:14:58 +0200 Subject: [PATCH 08/15] C++: Post-processing query for inline test expectations --- .../TestUtilities/InlineExpectationsTest.qll | 28 +-------------- .../InlineExpectationsTestQuery.ql | 21 ++++++++++++ .../internal/InlineExpectationsTestImpl.qll | 28 +++++++++++++++ .../Critical/SizeCheck/SizeCheck.qlref | 3 +- .../query-tests/Critical/SizeCheck/test.c | 14 ++++---- .../CWE-022/semmle/tests/TaintedPath.qlref | 3 +- .../Security/CWE/CWE-022/semmle/tests/test.c | 34 +++++++++---------- 7 files changed, 78 insertions(+), 53 deletions(-) create mode 100644 cpp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql create mode 100644 cpp/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll diff --git a/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll b/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll index 9f839de821b7..5d74a9da2c21 100644 --- a/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll +++ b/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -5,31 +5,5 @@ import cpp as C private import codeql.util.test.InlineExpectationsTest - -private module Impl implements InlineExpectationsTestSig { - private newtype TExpectationComment = MkExpectationComment(C::CppStyleComment c) - - /** - * A class representing a line comment in the CPP style. - * Unlike the `CppStyleComment` class, however, the string returned by `getContents` does _not_ - * include the preceding comment marker (`//`). - */ - class ExpectationComment extends TExpectationComment { - C::CppStyleComment comment; - - ExpectationComment() { this = MkExpectationComment(comment) } - - /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */ - string getContents() { result = comment.getContents().suffix(2) } - - /** Gets a textual representation of this element. */ - string toString() { result = comment.toString() } - - /** Gets the location of this comment. */ - Location getLocation() { result = comment.getLocation() } - } - - class Location = C::Location; -} - +private import internal.InlineExpectationsTestImpl import Make diff --git a/cpp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/cpp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..8e6977ba5321 --- /dev/null +++ b/cpp/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import cpp +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/cpp/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll b/cpp/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll new file mode 100644 index 000000000000..d2c8efbf3165 --- /dev/null +++ b/cpp/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll @@ -0,0 +1,28 @@ +import cpp as C +private import codeql.util.test.InlineExpectationsTest + +module Impl implements InlineExpectationsTestSig { + private newtype TExpectationComment = MkExpectationComment(C::CppStyleComment c) + + /** + * A class representing a line comment in the CPP style. + * Unlike the `CppStyleComment` class, however, the string returned by `getContents` does _not_ + * include the preceding comment marker (`//`). + */ + class ExpectationComment extends TExpectationComment { + C::CppStyleComment comment; + + ExpectationComment() { this = MkExpectationComment(comment) } + + /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */ + string getContents() { result = comment.getContents().suffix(2) } + + /** Gets a textual representation of this element. */ + string toString() { result = comment.toString() } + + /** Gets the location of this comment. */ + Location getLocation() { result = comment.getLocation() } + } + + class Location = C::Location; +} diff --git a/cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck.qlref b/cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck.qlref index 772c4cee2f9c..a1ab57f2b750 100644 --- a/cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck.qlref +++ b/cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck.qlref @@ -1 +1,2 @@ -Critical/SizeCheck.ql +query: Critical/SizeCheck.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Critical/SizeCheck/test.c b/cpp/ql/test/query-tests/Critical/SizeCheck/test.c index 0015a2514c08..b4ea2dc3f98f 100644 --- a/cpp/ql/test/query-tests/Critical/SizeCheck/test.c +++ b/cpp/ql/test/query-tests/Critical/SizeCheck/test.c @@ -13,8 +13,8 @@ void free(void *ptr); void bad0(void) { - float *fptr = malloc(3); // BAD -- Too small - double *dptr = malloc(5); // BAD -- Too small + float *fptr = malloc(3); // $ Alert -- Too small + double *dptr = malloc(5); // $ Alert -- Too small free(fptr); free(dptr); } @@ -29,8 +29,8 @@ void good0(void) { void bad1(void) { - float *fptr = malloc(sizeof(short)); // BAD -- Too small - double *dptr = malloc(sizeof(float)); // BAD -- Too small + float *fptr = malloc(sizeof(short)); // $ Alert -- Too small + double *dptr = malloc(sizeof(float)); // $ Alert -- Too small free(fptr); free(dptr); } @@ -56,7 +56,7 @@ typedef union _myUnion void test_union() { MyUnion *a = malloc(sizeof(MyUnion)); // GOOD - MyUnion *b = malloc(sizeof(MyStruct)); // BAD (too small) + MyUnion *b = malloc(sizeof(MyStruct)); // $ Alert (too small) } // --- custom allocators --- @@ -66,6 +66,6 @@ void *MyMalloc2(size_t size); void customAllocatorTests() { - float *fptr1 = MyMalloc1(3); // BAD (too small) [NOT DETECTED] - float *fptr2 = MyMalloc2(3); // BAD (too small) [NOT DETECTED] + float *fptr1 = MyMalloc1(3); // $ MISSING: BAD (too small) + float *fptr2 = MyMalloc2(3); // $ MISSING: BAD (too small) } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.qlref b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.qlref index 1677939387da..db270a97f3e2 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.qlref +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.qlref @@ -1 +1,2 @@ -Security/CWE/CWE-022/TaintedPath.ql \ No newline at end of file +query: Security/CWE/CWE-022/TaintedPath.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c index 4324f269df65..9185b02f0c4a 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c @@ -5,7 +5,7 @@ #define PATH_MAX 4096 ///// Test code ///// -int main(int argc, char** argv) { +int main(int argc, char** argv) { // $ Source=argv char *userAndFile = argv[2]; { @@ -14,7 +14,7 @@ int main(int argc, char** argv) { size_t len = strlen(fileName); strncat(fileName+len, userAndFile, FILENAME_MAX-len-1); // BAD: a string from the user is used in a filename - fopen(fileName, "wb+"); + fopen(fileName, "wb+"); // $ Alert=argv } { @@ -29,30 +29,30 @@ int main(int argc, char** argv) { { char *fileName = argv[1]; - fopen(fileName, "wb+"); // BAD + fopen(fileName, "wb+"); // $ Alert=argv } { char fileName[20]; - scanf("%s", fileName); - fopen(fileName, "wb+"); // BAD + scanf("%s", fileName); // $ Source=scanf_output1 + fopen(fileName, "wb+"); // $ Alert=scanf_output1 } { char *fileName = (char*)malloc(20 * sizeof(char)); - scanf("%s", fileName); - fopen(fileName, "wb+"); // BAD + scanf("%s", fileName); // $ Source=scanf_output2 + fopen(fileName, "wb+"); // $ Alert=scanf_output2 } { - char *tainted = getenv("A_STRING"); - fopen(tainted, "wb+"); // BAD + char *tainted = getenv("A_STRING"); // $ Source=getenv1 + fopen(tainted, "wb+"); // $ Alert=getenv1 } { char buffer[1024]; - strncpy(buffer, getenv("A_STRING"), 1024); - fopen(buffer, "wb+"); // BAD + strncpy(buffer, getenv("A_STRING"), 1024); // $ Source=getenv2 + fopen(buffer, "wb+"); // $ Alert=getenv2 fopen(buffer, "wb+"); // (we don't want a duplicate result here) } @@ -66,14 +66,14 @@ int main(int argc, char** argv) { { void readFile(const char *fileName); - readFile(argv[1]); // BAD + readFile(argv[1]); // $ Alert=argv } { char buffer[1024]; - read(0, buffer, 1024); - read(0, buffer, 1024); - fopen(buffer, "wb+"); // BAD [duplicated with both sources] + read(0, buffer, 1024); // $ Source=read_output1 + read(0, buffer, 1024); // $ Source=read_output2 + fopen(buffer, "wb+"); // $ Alert=read_output1 $ Alert=read_output2 } { @@ -81,7 +81,7 @@ int main(int argc, char** argv) { char fileBuffer[PATH_MAX]; snprintf(fileBuffer, sizeof(fileBuffer), "/home/%s", userAndFile); // BAD: a string from the user is used in a filename - fopen(fileBuffer, "wb+"); + fopen(fileBuffer, "wb+"); // $ Alert=argv } { @@ -95,7 +95,7 @@ int main(int argc, char** argv) { char fileBuffer[PATH_MAX]; snprintf(fileBuffer, sizeof(fileBuffer), "/home/user/files/%s", fileName); // GOOD: We know that the filename is safe and stays within the public folder. But we currently get an FP here. - FILE *file = fopen(fileBuffer, "wb+"); + FILE *file = fopen(fileBuffer, "wb+"); // $ SPURIOUS: Alert=argv } { From e5f2bbb6ec305dc9a47d7e28599b16ed3f7aaeeb Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 14:20:28 +0200 Subject: [PATCH 09/15] Python: Post-processing query for inline test expectations --- .../TestUtilities/InlineExpectationsTest.qll | 12 +---------- .../InlineExpectationsTestQuery.ql | 21 +++++++++++++++++++ .../internal/InlineExpectationsTestImpl.qll | 12 +++++++++++ .../query-tests/Numerics/Pythagorean.qlref | 3 ++- .../query-tests/Numerics/pythagorean_test.py | 6 +++--- .../CWE-094-CodeInjection/CodeInjection.qlref | 3 ++- .../CWE-094-CodeInjection/code_injection.py | 10 ++++----- 7 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 python/ql/test/TestUtilities/InlineExpectationsTestQuery.ql create mode 100644 python/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll diff --git a/python/ql/test/TestUtilities/InlineExpectationsTest.qll b/python/ql/test/TestUtilities/InlineExpectationsTest.qll index 40ed5d47efb8..ad671cbef39c 100644 --- a/python/ql/test/TestUtilities/InlineExpectationsTest.qll +++ b/python/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -5,15 +5,5 @@ private import python as PY private import codeql.util.test.InlineExpectationsTest - -private module Impl implements InlineExpectationsTestSig { - /** - * A class representing line comments in Python. As this is the only form of comment Python - * permits, we simply reuse the `Comment` class. - */ - class ExpectationComment = PY::Comment; - - class Location = PY::Location; -} - +private import internal.InlineExpectationsTestImpl import Make diff --git a/python/ql/test/TestUtilities/InlineExpectationsTestQuery.ql b/python/ql/test/TestUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..9ce5fdf326ca --- /dev/null +++ b/python/ql/test/TestUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import python +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} diff --git a/python/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll b/python/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll new file mode 100644 index 000000000000..ea8faaeeae31 --- /dev/null +++ b/python/ql/test/TestUtilities/internal/InlineExpectationsTestImpl.qll @@ -0,0 +1,12 @@ +private import python as PY +private import codeql.util.test.InlineExpectationsTest + +module Impl implements InlineExpectationsTestSig { + /** + * A class representing line comments in Python. As this is the only form of comment Python + * permits, we simply reuse the `Comment` class. + */ + class ExpectationComment = PY::Comment; + + class Location = PY::Location; +} diff --git a/python/ql/test/query-tests/Numerics/Pythagorean.qlref b/python/ql/test/query-tests/Numerics/Pythagorean.qlref index bc7326b415ab..541bd35ac626 100644 --- a/python/ql/test/query-tests/Numerics/Pythagorean.qlref +++ b/python/ql/test/query-tests/Numerics/Pythagorean.qlref @@ -1 +1,2 @@ -Numerics/Pythagorean.ql \ No newline at end of file +query: Numerics/Pythagorean.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Numerics/pythagorean_test.py b/python/ql/test/query-tests/Numerics/pythagorean_test.py index 2503a1d6c22f..6dd005b55b34 100644 --- a/python/ql/test/query-tests/Numerics/pythagorean_test.py +++ b/python/ql/test/query-tests/Numerics/pythagorean_test.py @@ -3,12 +3,12 @@ from math import sqrt def withPow(a, b): - return sqrt(a**2 + b**2) + return sqrt(a**2 + b**2) # $ Alert def withMul(a, b): - return sqrt(a*a + b*b) + return sqrt(a*a + b*b) # $ Alert def withRef(a, b): a2 = a**2 b2 = b*b - return sqrt(a2 + b2) \ No newline at end of file + return sqrt(a2 + b2) # $ Alert \ No newline at end of file diff --git a/python/ql/test/query-tests/Security/CWE-094-CodeInjection/CodeInjection.qlref b/python/ql/test/query-tests/Security/CWE-094-CodeInjection/CodeInjection.qlref index fe9adbf3b64d..0135c6787d4b 100644 --- a/python/ql/test/query-tests/Security/CWE-094-CodeInjection/CodeInjection.qlref +++ b/python/ql/test/query-tests/Security/CWE-094-CodeInjection/CodeInjection.qlref @@ -1 +1,2 @@ -Security/CWE-094/CodeInjection.ql +query: Security/CWE-094/CodeInjection.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql diff --git a/python/ql/test/query-tests/Security/CWE-094-CodeInjection/code_injection.py b/python/ql/test/query-tests/Security/CWE-094-CodeInjection/code_injection.py index 05dabe166cf6..c775d01250cb 100644 --- a/python/ql/test/query-tests/Security/CWE-094-CodeInjection/code_injection.py +++ b/python/ql/test/query-tests/Security/CWE-094-CodeInjection/code_injection.py @@ -1,13 +1,13 @@ -from flask import Flask, request +from flask import Flask, request # $ Source=flask app = Flask(__name__) @app.route("/code-execution") def code_execution(): code = request.args.get("code") - exec(code) # NOT OK - eval(code) # NOT OK + exec(code) # $ Alert=flask + eval(code) # $ Alert=flask cmd = compile(code, "", "exec") - exec(cmd) # NOT OK + exec(cmd) # $ Alert=flask @app.route("/safe-code-execution") @@ -18,5 +18,5 @@ def code_execution(): obj_name = request.args.get("obj") if obj_name == "foo" or obj_name == "bar": # TODO: Should not alert on this - obj = eval(obj_name) # OK + obj = eval(obj_name) # $ SPURIOUS: Alert=flask print(obj, obj*10) From 1259b7e8e7734381b5580e3e18ed69d9d677d83f Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 23 Sep 2024 14:24:37 +0200 Subject: [PATCH 10/15] JS: Post-processing query for inline test expectations --- .../query-tests/Security/CWE-611/Xxe.qlref | 3 ++- .../query-tests/Security/CWE-611/domparser.js | 6 +++--- .../Security/CWE-611/libxml.noent.js | 16 +++++++------- .../Security/CWE-611/libxml.sax.js | 4 ++-- .../Security/CWE-611/libxml.saxpush.js | 4 ++-- .../InlineExpectationsTestQuery.ql | 21 +++++++++++++++++++ 6 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 javascript/ql/test/testUtilities/InlineExpectationsTestQuery.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-611/Xxe.qlref b/javascript/ql/test/query-tests/Security/CWE-611/Xxe.qlref index 62a3f7f22d97..38e346c1a8b2 100644 --- a/javascript/ql/test/query-tests/Security/CWE-611/Xxe.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-611/Xxe.qlref @@ -1 +1,2 @@ -Security/CWE-611/Xxe.ql +query: Security/CWE-611/Xxe.ql +postprocess: testUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-611/domparser.js b/javascript/ql/test/query-tests/Security/CWE-611/domparser.js index dca7a3333e7b..eeb2236a1d12 100644 --- a/javascript/ql/test/query-tests/Security/CWE-611/domparser.js +++ b/javascript/ql/test/query-tests/Security/CWE-611/domparser.js @@ -1,5 +1,5 @@ function test() { - var src = document.location.search; + var src = document.location.search; // $ Source=search if (window.DOMParser) { // OK: DOMParser only expands internal general entities @@ -8,10 +8,10 @@ function test() { var parser; try { // NOT OK: XMLDOM expands external entities by default - (new ActiveXObject("Microsoft.XMLDOM")).loadXML(src); + (new ActiveXObject("Microsoft.XMLDOM")).loadXML(src); // $ Alert=search } catch (e) { // NOT OK: MSXML expands external entities by default - (new ActiveXObject("Msxml2.DOMDocument")).loadXML(src); + (new ActiveXObject("Msxml2.DOMDocument")).loadXML(src); // $ Alert=search } } } diff --git a/javascript/ql/test/query-tests/Security/CWE-611/libxml.noent.js b/javascript/ql/test/query-tests/Security/CWE-611/libxml.noent.js index 47df4223e0e4..40cb0148b573 100644 --- a/javascript/ql/test/query-tests/Security/CWE-611/libxml.noent.js +++ b/javascript/ql/test/query-tests/Security/CWE-611/libxml.noent.js @@ -1,20 +1,20 @@ const express = require('express'); const libxmljs = require('libxmljs'); -express().get('/some/path', function(req) { +express().get('/some/path', function (req) { // NOT OK: unguarded entity expansion - libxmljs.parseXml(req.param("some-xml"), { noent: true }); + libxmljs.parseXml(req.param("some-xml"), { noent: true }); // $ Alert }); -express().post('/some/path', function(req, res) { +express().post('/some/path', function (req, res) { // NOT OK: unguarded entity expansion - libxmljs.parseXml(req.param("some-xml"), { noent: true }); + libxmljs.parseXml(req.param("some-xml"), { noent: true }); // $ Alert // NOT OK: unguarded entity expansion - libxmljs.parseXmlString(req.param("some-xml"), {noent:true}) + libxmljs.parseXmlString(req.param("some-xml"), { noent: true }) // $ Alert // NOT OK: unguarded entity expansion - libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:true}) - + libxmljs.parseXmlString(req.files.products.data.toString('utf8'), { noent: true })// $ Source=files $ Alert=files + // OK - no entity expansion - libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:false}) + libxmljs.parseXmlString(req.files.products.data.toString('utf8'), { noent: false }) }); diff --git a/javascript/ql/test/query-tests/Security/CWE-611/libxml.sax.js b/javascript/ql/test/query-tests/Security/CWE-611/libxml.sax.js index 2c837c750d32..ba5723c794b1 100644 --- a/javascript/ql/test/query-tests/Security/CWE-611/libxml.sax.js +++ b/javascript/ql/test/query-tests/Security/CWE-611/libxml.sax.js @@ -1,7 +1,7 @@ const express = require('express'); const libxmljs = require('libxmljs'); -express().get('/some/path', function(req) { +express().get('/some/path', function (req) { const parser = new libxmljs.SaxParser(); - parser.parseString(req.param("some-xml")); // NOT OK: the SAX parser expands external entities by default + parser.parseString(req.param("some-xml")); // $ Alert: the SAX parser expands external entities by default }); diff --git a/javascript/ql/test/query-tests/Security/CWE-611/libxml.saxpush.js b/javascript/ql/test/query-tests/Security/CWE-611/libxml.saxpush.js index 0e939e191c62..f8106b378038 100644 --- a/javascript/ql/test/query-tests/Security/CWE-611/libxml.saxpush.js +++ b/javascript/ql/test/query-tests/Security/CWE-611/libxml.saxpush.js @@ -1,7 +1,7 @@ const express = require('express'); const libxmljs = require('libxmljs'); -express().get('/some/path', function(req) { +express().get('/some/path', function (req) { const parser = new libxmljs.SaxPushParser(); - parser.push(req.param("some-xml")); // NOT OK: the SAX parser expands external entities by default + parser.push(req.param("some-xml")); // $ Alert: the SAX parser expands external entities by default }); diff --git a/javascript/ql/test/testUtilities/InlineExpectationsTestQuery.ql b/javascript/ql/test/testUtilities/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..55892be75d79 --- /dev/null +++ b/javascript/ql/test/testUtilities/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import javascript +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} From dd520fea4729e3426d593aa679203b90976537ff Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 7 Oct 2024 13:12:19 +0200 Subject: [PATCH 11/15] Rust: Post-processing query for inline test expectations --- .../unusedentities/UnreachableCode.expected | 32 +- .../unusedentities/UnreachableCode.qlref | 3 +- .../unusedentities/UnusedValue.expected | 22 +- .../unusedentities/UnusedValue.qlref | 3 +- .../unusedentities/UnusedVariable.expected | 38 +- .../unusedentities/UnusedVariable.qlref | 3 +- .../test/query-tests/unusedentities/main.rs | 165 +++---- .../test/query-tests/unusedentities/more.rs | 30 +- .../query-tests/unusedentities/unreachable.rs | 414 +++++++++--------- .../test/utils/InlineExpectationsTestQuery.ql | 21 + 10 files changed, 368 insertions(+), 363 deletions(-) create mode 100644 rust/ql/test/utils/InlineExpectationsTestQuery.ql diff --git a/rust/ql/test/query-tests/unusedentities/UnreachableCode.expected b/rust/ql/test/query-tests/unusedentities/UnreachableCode.expected index 3ae936d9463d..f2e266021aa6 100644 --- a/rust/ql/test/query-tests/unusedentities/UnreachableCode.expected +++ b/rust/ql/test/query-tests/unusedentities/UnreachableCode.expected @@ -1,16 +1,16 @@ -| unreachable.rs:13:3:13:17 | ExprStmt | This code is never reached. | -| unreachable.rs:21:3:21:17 | ExprStmt | This code is never reached. | -| unreachable.rs:33:3:33:17 | ExprStmt | This code is never reached. | -| unreachable.rs:40:3:40:17 | ExprStmt | This code is never reached. | -| unreachable.rs:61:2:61:16 | ExprStmt | This code is never reached. | -| unreachable.rs:107:16:107:23 | ExprStmt | This code is never reached. | -| unreachable.rs:115:15:115:22 | ExprStmt | This code is never reached. | -| unreachable.rs:141:2:141:16 | ExprStmt | This code is never reached. | -| unreachable.rs:148:3:148:17 | ExprStmt | This code is never reached. | -| unreachable.rs:157:4:157:18 | ExprStmt | This code is never reached. | -| unreachable.rs:163:3:163:17 | ExprStmt | This code is never reached. | -| unreachable.rs:169:4:169:18 | ExprStmt | This code is never reached. | -| unreachable.rs:177:4:177:18 | ExprStmt | This code is never reached. | -| unreachable.rs:180:2:180:16 | ExprStmt | This code is never reached. | -| unreachable.rs:203:3:203:17 | ExprStmt | This code is never reached. | -| unreachable.rs:218:3:218:17 | ExprStmt | This code is never reached. | +| unreachable.rs:11:9:11:23 | ExprStmt | This code is never reached. | +| unreachable.rs:19:9:19:23 | ExprStmt | This code is never reached. | +| unreachable.rs:31:9:31:23 | ExprStmt | This code is never reached. | +| unreachable.rs:38:9:38:23 | ExprStmt | This code is never reached. | +| unreachable.rs:59:5:59:19 | ExprStmt | This code is never reached. | +| unreachable.rs:106:13:106:20 | ExprStmt | This code is never reached. | +| unreachable.rs:115:13:115:20 | ExprStmt | This code is never reached. | +| unreachable.rs:141:5:141:19 | ExprStmt | This code is never reached. | +| unreachable.rs:148:9:148:23 | ExprStmt | This code is never reached. | +| unreachable.rs:157:13:157:27 | ExprStmt | This code is never reached. | +| unreachable.rs:163:9:163:23 | ExprStmt | This code is never reached. | +| unreachable.rs:169:13:169:27 | ExprStmt | This code is never reached. | +| unreachable.rs:177:13:177:27 | ExprStmt | This code is never reached. | +| unreachable.rs:180:5:180:19 | ExprStmt | This code is never reached. | +| unreachable.rs:204:9:204:23 | ExprStmt | This code is never reached. | +| unreachable.rs:220:9:220:23 | ExprStmt | This code is never reached. | diff --git a/rust/ql/test/query-tests/unusedentities/UnreachableCode.qlref b/rust/ql/test/query-tests/unusedentities/UnreachableCode.qlref index f65928931a13..23ca9359181e 100644 --- a/rust/ql/test/query-tests/unusedentities/UnreachableCode.qlref +++ b/rust/ql/test/query-tests/unusedentities/UnreachableCode.qlref @@ -1 +1,2 @@ -queries/unusedentities/UnreachableCode.ql \ No newline at end of file +query: queries/unusedentities/UnreachableCode.ql +postprocess: utils/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected index 1ee928c92be2..e2d0fa11faab 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected +++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected @@ -10,15 +10,15 @@ | main.rs:65:5:65:5 | g | Variable is assigned a value that is never used. | | main.rs:87:9:87:9 | a | Variable is assigned a value that is never used. | | main.rs:108:9:108:10 | is | Variable is assigned a value that is never used. | -| main.rs:133:13:133:17 | total | Variable is assigned a value that is never used. | -| main.rs:203:13:203:31 | res | Variable is assigned a value that is never used. | -| main.rs:218:9:218:24 | kind | Variable is assigned a value that is never used. | -| main.rs:223:9:223:32 | kind | Variable is assigned a value that is never used. | -| main.rs:280:13:280:17 | total | Variable is assigned a value that is never used. | -| main.rs:348:5:348:39 | kind | Variable is assigned a value that is never used. | -| main.rs:370:9:370:9 | x | Variable is assigned a value that is never used. | -| main.rs:378:17:378:17 | x | Variable is assigned a value that is never used. | +| main.rs:131:13:131:17 | total | Variable is assigned a value that is never used. | +| main.rs:194:13:194:31 | res | Variable is assigned a value that is never used. | +| main.rs:206:9:206:24 | kind | Variable is assigned a value that is never used. | +| main.rs:210:9:210:32 | kind | Variable is assigned a value that is never used. | +| main.rs:266:13:266:17 | total | Variable is assigned a value that is never used. | +| main.rs:334:5:334:39 | kind | Variable is assigned a value that is never used. | +| main.rs:359:9:359:9 | x | Variable is assigned a value that is never used. | +| main.rs:367:17:367:17 | x | Variable is assigned a value that is never used. | | more.rs:24:9:24:11 | val | Variable is assigned a value that is never used. | -| more.rs:46:9:46:14 | a_ptr4 | Variable is assigned a value that is never used. | -| more.rs:61:9:61:13 | d_ptr | Variable is assigned a value that is never used. | -| more.rs:67:9:67:17 | f_ptr | Variable is assigned a value that is never used. | +| more.rs:44:9:44:14 | a_ptr4 | Variable is assigned a value that is never used. | +| more.rs:59:9:59:13 | d_ptr | Variable is assigned a value that is never used. | +| more.rs:65:9:65:17 | f_ptr | Variable is assigned a value that is never used. | diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.qlref b/rust/ql/test/query-tests/unusedentities/UnusedValue.qlref index d5ee4e655157..d08b310e2d04 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedValue.qlref +++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.qlref @@ -1 +1,2 @@ -queries/unusedentities/UnusedValue.ql \ No newline at end of file +query: queries/unusedentities/UnusedValue.ql +postprocess: utils/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected index 3a919734adee..804a0849e7aa 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected +++ b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected @@ -1,21 +1,21 @@ | main.rs:25:9:25:9 | a | Variable is not used. | | main.rs:90:13:90:13 | d | Variable is not used. | -| main.rs:141:5:141:5 | y | Variable is not used. | -| main.rs:168:9:168:9 | x | Variable is not used. | -| main.rs:250:17:250:17 | a | Variable is not used. | -| main.rs:258:20:258:22 | val | Variable is not used. | -| main.rs:272:14:272:16 | val | Variable is not used. | -| main.rs:287:22:287:24 | val | Variable is not used. | -| main.rs:294:24:294:26 | val | Variable is not used. | -| main.rs:302:13:302:15 | num | Variable is not used. | -| main.rs:317:12:317:12 | j | Variable is not used. | -| main.rs:337:25:337:25 | y | Variable is not used. | -| main.rs:340:28:340:28 | a | Variable is not used. | -| main.rs:343:9:343:9 | p | Variable is not used. | -| main.rs:358:9:358:13 | right | Variable is not used. | -| main.rs:364:9:364:14 | right2 | Variable is not used. | -| main.rs:371:13:371:13 | y | Variable is not used. | -| main.rs:379:21:379:21 | y | Variable is not used. | -| main.rs:427:27:427:29 | val | Variable is not used. | -| main.rs:430:22:430:24 | acc | Variable is not used. | -| main.rs:455:9:455:14 | unused | Variable is not used. | +| main.rs:139:5:139:5 | y | Variable is not used. | +| main.rs:166:9:166:9 | x | Variable is not used. | +| main.rs:236:17:236:17 | a | Variable is not used. | +| main.rs:244:20:244:22 | val | Variable is not used. | +| main.rs:258:14:258:16 | val | Variable is not used. | +| main.rs:273:22:273:24 | val | Variable is not used. | +| main.rs:280:24:280:26 | val | Variable is not used. | +| main.rs:288:13:288:15 | num | Variable is not used. | +| main.rs:303:12:303:12 | j | Variable is not used. | +| main.rs:323:25:323:25 | y | Variable is not used. | +| main.rs:326:28:326:28 | a | Variable is not used. | +| main.rs:329:9:329:9 | p | Variable is not used. | +| main.rs:347:9:347:13 | right | Variable is not used. | +| main.rs:353:9:353:14 | right2 | Variable is not used. | +| main.rs:360:13:360:13 | y | Variable is not used. | +| main.rs:368:21:368:21 | y | Variable is not used. | +| main.rs:413:26:413:28 | val | Variable is not used. | +| main.rs:416:21:416:23 | acc | Variable is not used. | +| main.rs:437:9:437:14 | unused | Variable is not used. | diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.qlref b/rust/ql/test/query-tests/unusedentities/UnusedVariable.qlref index a1cd7d0a904b..709d2b61d550 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedVariable.qlref +++ b/rust/ql/test/query-tests/unusedentities/UnusedVariable.qlref @@ -1 +1,2 @@ -queries/unusedentities/UnusedVariable.ql \ No newline at end of file +query: queries/unusedentities/UnusedVariable.ql +postprocess: utils/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs index e78e8daed0e6..07485e3af714 100644 --- a/rust/ql/test/query-tests/unusedentities/main.rs +++ b/rust/ql/test/query-tests/unusedentities/main.rs @@ -3,10 +3,10 @@ // --- locals --- fn locals_1() { - let a = 1; // BAD: unused value + let a = 1; // $ Alert[rust/unused-value] let b = 1; let c = 1; - let d = String::from("a"); // BAD: unused value + let d = String::from("a"); // $ Alert[rust/unused-value] let e = String::from("b"); let f = 1; let _ = 1; // (deliberately unused) @@ -22,7 +22,7 @@ fn locals_1() { } fn locals_2() { - let a: i32; // BAD: unused variable + let a: i32; // $ Alert[rust/unused-variable] let b: i32; let mut c: i32; let mut d: i32; @@ -32,22 +32,22 @@ fn locals_2() { let h: i32; let i: i32; - b = 1; // BAD: unused value + b = 1; // $ Alert[rust/unused-value] - c = 1; // BAD: unused value + c = 1; // $ Alert[rust/unused-value] c = 2; println!("use {}", c); - c = 3; // BAD: unused value + c = 3; // $ Alert[rust/unused-value] d = 1; if cond() { - d = 2; // BAD: unused value + d = 2; // $ Alert[rust/unused-value] d = 3; } else { } println!("use {}", d); - e = 1; // BAD: unused value + e = 1; // $ Alert[rust/unused-value] if cond() { e = 2; } else { @@ -58,11 +58,11 @@ fn locals_2() { f = 1; f += 1; println!("use {}", f); - f += 1; // BAD: unused value + f += 1; // $ Alert[rust/unused-value] f = 1; - f += 1; // BAD: unused value + f += 1; // $ Alert[rust/unused-value] - g = if cond() { 1 } else { 2 }; // BAD: unused value + g = if cond() { 1 } else { 2 }; // $ Alert[rust/unused-value] h = if cond() { 3 } else { 4 }; i = if cond() { h } else { 5 }; println!("use {}", i); @@ -84,10 +84,10 @@ impl MyStruct { } fn structs() { - let a = MyStruct { val: 1 }; // BAD: unused value + let a = MyStruct { val: 1 }; // $ Alert[rust/unused-value] let b = MyStruct { val: 2 }; let c = MyStruct { val: 3 }; - let mut d: MyStruct; // BAD: unused variable + let mut d: MyStruct; // $ Alert[rust/unused-variable] let mut e: MyStruct; let mut f: MyStruct; @@ -98,22 +98,20 @@ fn structs() { e.val = 5; println!("lets use {}", e.my_get()); - f = MyStruct { val: 6 }; // BAD: unused value [NOT DETECTED] - f.val = 7; // BAD: unused value [NOT DETECTED] + f = MyStruct { val: 6 }; // $ MISSING: Alert[rust/unused-value] + f.val = 7; // $ MISSING: Alert[rust/unused-value] } // --- arrays --- fn arrays() { - let is = [1, 2, 3]; // BAD: unused value + let is = [1, 2, 3]; // $ Alert[rust/unused-value] let js = [1, 2, 3]; let ks = [1, 2, 3]; println!("lets use {:?}", js); - for k - in ks - { + for k in ks { println!("lets use {}", k); // [unreachable FALSE POSITIVE] } } @@ -121,16 +119,16 @@ fn arrays() { // --- constants and statics --- const CON1: i32 = 1; -const CON2: i32 = 2; // BAD: unused value [NOT DETECTED] +const CON2: i32 = 2; // $ MISSING: Alert[rust/unused-value] static mut STAT1: i32 = 1; -static mut STAT2: i32 = 2; // BAD: unused value [NOT DETECTED] +static mut STAT2: i32 = 2; // $ MISSING: Alert[rust/unused-value] fn statics() { static mut STAT3: i32 = 0; - static mut STAT4: i32 = 0; // BAD: unused value [NOT DETECTED] + static mut STAT4: i32 = 0; // $ MISSING: Alert[rust/unused-value] unsafe { - let total = CON1 + STAT1 + STAT3; // BAD: unused value + let total = CON1 + STAT1 + STAT3; // $ Alert[rust/unused-value] } } @@ -138,7 +136,7 @@ fn statics() { fn parameters( x: i32, - y: i32, // BAD: unused variable + y: i32, // $ Alert[rust/unused-variable] _z: i32, // (`_` is asking the compiler, and by extension us, to not warn that this is unused) ) -> i32 { return x; @@ -165,64 +163,52 @@ fn loops() { e += x; } - for x in 1..10 { // BAD: unused variable + for x in 1..10 { // $ Alert[rust/unused-variable] } for _ in 1..10 {} - for x - in 1..10 { + for x in 1..10 { println!("x is {}", x); } - for x - in 1..10 { + for x in 1..10 { println!("x is {:?}", x); } - for x - in 1..10 { + for x in 1..10 { println!("x + 1 is {}", x + 1); } - for x - in 1..10 { - for y - in 1..x { + for x in 1..10 { + for y in 1..x { println!("y is {}", y); } } - for x - in 1..10 { + for x in 1..10 { println!("x is {x}"); } - for x - in 1..10 { - _ = format!("x is {x}"); // SPURIOUS: unused value `res` + for x in 1..10 { + _ = format!("x is {x}"); // $ SPURIOUS: Alert[rust/unused-value] } - for x - in 1..10 { + for x in 1..10 { println!("x is {val}", val = x); } - for x - in 1..10 { + for x in 1..10 { assert!(x != 11); } - for x - in 1..10 { - assert_eq!(x, 1); // SPURIOUS: unused value `kind` + for x in 1..10 { + assert_eq!(x, 1); // $ SPURIOUS: Alert[rust/unused-value] } - for x - in 1..10 { - assert_eq!(id(x), id(1)); // SPURIOUS: unused value `kind` + for x in 1..10 { + assert_eq!(id(x), id(1)); // $ SPURIOUS: Alert[rust/unused-value] } - } // --- lets --- @@ -237,7 +223,7 @@ enum YesOrNo { No, } -use YesOrNo::{Yes, No}; // allows `Yes`, `No` to be accessed without qualifiers. +use YesOrNo::{No, Yes}; // allows `Yes`, `No` to be accessed without qualifiers. struct MyPoint { x: i64, @@ -247,7 +233,7 @@ struct MyPoint { fn if_lets_matches() { let mut total: i64 = 0; - if let Some(a) = Some(10) { // BAD: unused variable + if let Some(a) = Some(10) { // $ Alert[rust/unused-variable] } if let Some(b) = Some(20) { @@ -255,7 +241,7 @@ fn if_lets_matches() { } let mut next = Some(30); - while let Some(val) = // BAD: unused variable + while let Some(val) = // $ Alert[rust/unused-variable] next { next = None; @@ -269,7 +255,7 @@ fn if_lets_matches() { let c = Some(60); match c { - Some(val) => { // BAD: unused variable + Some(val) => { // $ Alert[rust/unused-variable] } None => {} } @@ -277,21 +263,21 @@ fn if_lets_matches() { let d = Some(70); match d { Some(val) => { - total += val; // BAD: unused value + total += val; // $ Alert[rust/unused-value] } None => {} } let e = Option::Some(80); match e { - Option::Some(val) => { // BAD: unused variable + Option::Some(val) => { // $ Alert[rust/unused-variable] } Option::None => {} } let f = MyOption::Some(90); match f { - MyOption::Some(val) => { // BAD: unused variable + MyOption::Some(val) => { // $ Alert[rust/unused-variable] } MyOption::None => {} } @@ -299,7 +285,7 @@ fn if_lets_matches() { let g: Result = Ok(100); match g { Ok(_) => {} - Err(num) => {} // BAD: unused variable + Err(num) => {} // $ Alert[rust/unused-variable] } let h = YesOrNo::Yes; @@ -314,7 +300,7 @@ fn if_lets_matches() { No => {} } - if let j = Yes { // BAD: unused variable + if let j = Yes { // $ Alert[rust/unused-variable] } if let k = Yes { @@ -334,49 +320,52 @@ fn if_lets_matches() { let p1 = MyPoint { x: 1, y: 2 }; match p1 { MyPoint { x: 0, y: 0 } => {} - MyPoint { x: 1, y } => { // BAD: unused variable + MyPoint { x: 1, y } => { // $ Alert[rust/unused-variable] } MyPoint { x: 2, y: _ } => {} - MyPoint { x: 3, y: a } => { // BAD: unused variable + MyPoint { x: 3, y: a } => { // $ Alert[rust/unused-variable] } MyPoint { x: 4, .. } => {} - p => { // BAD: unused variable + p => { // $ Alert[rust/unused-variable] } } let duration1 = std::time::Duration::new(10, 0); // ten seconds - assert_eq!(duration1.as_secs(), 10); // SPURIOUS: unused value `kind` + assert_eq!(duration1.as_secs(), 10); // $ SPURIOUS: Alert[rust/unused-value] - let duration2:Result = - Ok(std::time::Duration::new(10, 0)); + let duration2: Result = Ok(std::time::Duration::new(10, 0)); match duration2 { - Ok(n) => { println!("duration was {} seconds", n.as_secs()); } - Err(_) => { println!("failed"); } + Ok(n) => { + println!("duration was {} seconds", n.as_secs()); + } + Err(_) => { + println!("failed"); + } } let (left, - right) = // BAD: unused value [NOT DETECTED] SPURIOUS: unused variable + right) = // $ MISSING: Alert[rust/unused-value] $ SPURIOUS: Alert[rust/unused-variable] (1, 2); _ = left; let pair = (1, 2); let (left2, - right2) = // BAD: unused value [NOT DETECTED] SPURIOUS: unused variable + right2) = // $ MISSING: Alert[rust/unused-value] $ SPURIOUS: Alert[rust/unused-variable] pair; _ = left2; } fn shadowing() -> i32 { - let x = 1; // BAD: unused value - let mut y: i32; // BAD: unused variable + let x = 1; // $ Alert[rust/unused-value] + let mut y: i32; // $ Alert[rust/unused-variable] { let x = 2; let mut y: i32; { - let x = 3; // BAD: unused value - let mut y: i32; // BAD: unused variable + let x = 3; // $ Alert[rust/unused-value] + let mut y: i32; // $ Alert[rust/unused-variable] } y = x; @@ -395,18 +384,15 @@ fn increment(x: i32) -> i32 { fn func_ptrs() { let my_func: FuncPtr = increment; - for x - in 1..10 { + for x in 1..10 { _ = x + 1; } - for x - in 1..10 { + for x in 1..10 { _ = increment(x); } - for x - in 1..10 { + for x in 1..10 { _ = my_func(x); } } @@ -418,30 +404,26 @@ fn folds_and_closures() { _ = a1.sum::(); let a2 = 1..10; - _ = a2.fold(0, | acc: i32, val: i32 | -> i32 { acc + val } ); + _ = a2.fold(0, |acc: i32, val: i32| -> i32 { acc + val }); let a3 = 1..10; - _ = a3.fold(0, | acc, val | acc + val); + _ = a3.fold(0, |acc, val| acc + val); let a4 = 1..10; - _ = a4.fold(0, | acc, val | acc); // BAD: unused variable + _ = a4.fold(0, |acc, val| acc); // $ Alert[rust/unused-variable] let a5 = 1..10; - _ = a5.fold(0, | acc, val | val); // BAD: unused variable + _ = a5.fold(0, |acc, val| val); // $ Alert[rust/unused-variable] let i6 = 1; let a6 = 1..10; - _ = a6.fold(0, | acc, val | acc + val + i6); + _ = a6.fold(0, |acc, val| acc + val + i6); } // --- traits --- trait Incrementable { - fn increment( - &mut self, - times: i32, - unused: &mut i32 - ); + fn increment(&mut self, times: i32, unused: &mut i32); } struct MyValue { @@ -452,7 +434,7 @@ impl Incrementable for MyValue { fn increment( &mut self, times: i32, - unused: i32 // BAD: unused variable + unused: i32, // $ Alert[rust/unused-variable] ) { self.value += times; } @@ -490,5 +472,4 @@ fn main() { unreachable_let_2(); unreachable_if_2(); unreachable_if_3(); - } diff --git a/rust/ql/test/query-tests/unusedentities/more.rs b/rust/ql/test/query-tests/unusedentities/more.rs index d8444da8f3f0..cb33d5f40fc7 100644 --- a/rust/ql/test/query-tests/unusedentities/more.rs +++ b/rust/ql/test/query-tests/unusedentities/more.rs @@ -9,7 +9,7 @@ trait MyGettable { } struct MyContainer { - val: T + val: T, } impl MySettable for MyContainer { @@ -21,19 +21,17 @@ impl MySettable for MyContainer { impl MyGettable for MyContainer { fn get( &self, - val: T // BAD: unused variable [NOT DETECTED] SPURIOUS: unused value + val: T, // $ SPURIOUS: Alert[rust/unused-value] $ MISSING: Alert[rust/unused-variable] ) -> &T { return &(self.val); } } fn generics() { - let mut a = MyContainer { val: 1 }; // BAD: unused value [NOT DETECTED] + let mut a = MyContainer { val: 1 }; // $ MISSING: Alert[rust/unused-value] let b = MyContainer { val: 2 }; - a.set( - *b.get(3) - ); + a.set(*b.get(3)); } // --- pointers --- @@ -42,13 +40,13 @@ fn pointers() { let a = 1; let a_ptr1 = &a; let a_ptr2 = &a; - let a_ptr3 = &a; // BAD: unused value [NOT DETECTED] - let a_ptr4 = &a; // BAD: unused value + let a_ptr3 = &a; // $ MISSING: Alert[rust/unused-value] + let a_ptr4 = &a; // $ Alert[rust/unused-value] println!("{}", *a_ptr1); println!("{}", a_ptr2); println!("{}", &a_ptr3); - let b = 2; // BAD: unused value [NOT DETECTED] + let b = 2; // $ MISSING: Alert[rust/unused-value] let b_ptr = &b; println!("{}", b_ptr); @@ -58,26 +56,26 @@ fn pointers() { println!("{}", **c_ptr_ptr); let d = 4; - let d_ptr = &d; // BAD: unused value + let d_ptr = &d; // $ Alert[rust/unused-value] let d_ptr_ptr = &&d; println!("{}", **d_ptr_ptr); - let e = 5; // BAD: unused value [NOT DETECTED] + let e = 5; // $ MISSING: Alert[rust/unused-value] let f = 6; - let mut f_ptr = &e; // BAD: unused value + let mut f_ptr = &e; // $ Alert[rust/unused-value] f_ptr = &f; println!("{}", *f_ptr); - let mut g = 7; // BAD: unused value [NOT DETECTED] + let mut g = 7; // $ MISSING: Alert[rust/unused-value] let g_ptr = &mut g; - *g_ptr = 77; // BAD: unused value [NOT DETECTED] + *g_ptr = 77; // $ MISSING: Alert[rust/unused-value] - let mut h = 8; // BAD: unused value [NOT DETECTED] + let mut h = 8; // $ MISSING: Alert[rust/unused-value] let h_ptr = &mut h; *h_ptr = 88; println!("{}", h); - let mut i = 9; // BAD: unused value [NOT DETECTED] + let mut i = 9; // $ MISSING: Alert[rust/unused-value] let i_ptr = &mut i; *i_ptr = 99; let i_ptr2 = &mut i; diff --git a/rust/ql/test/query-tests/unusedentities/unreachable.rs b/rust/ql/test/query-tests/unusedentities/unreachable.rs index e7c252db8ac5..6bc064e145e4 100644 --- a/rust/ql/test/query-tests/unusedentities/unreachable.rs +++ b/rust/ql/test/query-tests/unusedentities/unreachable.rs @@ -1,243 +1,245 @@ - //fn cond() -> bool; //fn get_a_number() -> i32; //fn maybe_get_a_number() -> Option; // --- unreachable code -- -fn do_something() { -} +fn do_something() {} fn unreachable_if_1() { - if false { - do_something(); // BAD: unreachable code - } else { - do_something(); - } - - if true { - do_something(); - } else { - do_something(); // BAD: unreachable code - } - - let v = get_a_number(); - if v == 1 { - if v != 1 { - do_something(); // BAD: unreachable code [NOT DETECTED] - } - } - - if cond() { - return; - do_something(); // BAD: unreachable code - } - - if cond() { - do_something(); - } else { - return; - do_something(); // BAD: unreachable code - } - do_something(); - - if cond() { - let x = cond(); - - if (x) { - do_something(); - if (!x) { - do_something(); // BAD: unreachable code [NOT DETECTED] - } - do_something(); - } - } - - if cond() { - return; - } else { - return; - } - do_something(); // BAD: unreachable code + if false { + do_something(); // $ Alert[rust/dead-code] + } else { + do_something(); + } + + if true { + do_something(); + } else { + do_something(); // $ Alert[rust/dead-code] + } + + let v = get_a_number(); + if v == 1 { + if v != 1 { + do_something(); // $ MISSING: Alert[rust/dead-code] + } + } + + if cond() { + return; + do_something(); // $ Alert[rust/dead-code] + } + + if cond() { + do_something(); + } else { + return; + do_something(); // $ Alert[rust/dead-code] + } + do_something(); + + if cond() { + let x = cond(); + + if (x) { + do_something(); + if (!x) { + do_something(); // $ MISSING: Alert[rust/dead-code] + } + do_something(); + } + } + + if cond() { + return; + } else { + return; + } + do_something(); // $ Alert[rust/dead-code] } fn unreachable_panic() { - if cond() { - do_something(); - panic!("Oh no!!!"); - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - do_something(); - unimplemented!(); - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - do_something(); - todo!(); - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - do_something(); - unreachable!(); - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - let mut maybe; - - maybe = Some("Thing"); - _ = maybe.unwrap(); // (safe) - do_something(); - - maybe = if cond() { Some("Other") } else { None }; - _ = maybe.unwrap(); // (might panic) - do_something(); - - maybe = None; - _ = maybe.unwrap(); // (always panics) - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - do_something(); - _ = false && panic!(); // does not panic due to short-circuiting SPURIOUS: unreachable - do_something(); - _ = false || panic!(); - do_something(); // BAD: unreachable code [NOT DETECTED] - } - - if cond() { - do_something(); - _ = true || panic!(); // does not panic due to short-circuiting SPURIOUS: unreachable - do_something(); - _ = true && panic!(); - do_something(); // BAD: unreachable code [NOT DETECTED] - } + if cond() { + do_something(); + panic!("Oh no!!!"); + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + do_something(); + unimplemented!(); + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + do_something(); + todo!(); + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + do_something(); + unreachable!(); + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + let mut maybe; + + maybe = Some("Thing"); + _ = maybe.unwrap(); // (safe) + do_something(); + + maybe = if cond() { Some("Other") } else { None }; + _ = maybe.unwrap(); // (might panic) + do_something(); + + maybe = None; + _ = maybe.unwrap(); // (always panics) + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + do_something(); + _ = false && // . + panic!(); // $ Alert[rust/dead-code] + do_something(); + _ = false || panic!(); + do_something(); // $ MISSING: Alert[rust/dead-code] + } + + if cond() { + do_something(); + _ = true || // . + panic!(); // $ Alert[rust/dead-code] + do_something(); + _ = true && panic!(); + do_something(); // $ MISSING: Alert[rust/dead-code] + } } fn unreachable_match() { - match get_a_number() { - 1=>{ - return; - } - _=>{ - do_something(); - } - } - do_something(); - - match get_a_number() { - 1=>{ - return; - } - _=>{ - return; - } - } - do_something(); // BAD: unreachable code + match get_a_number() { + 1 => { + return; + } + _ => { + do_something(); + } + } + do_something(); + + match get_a_number() { + 1 => { + return; + } + _ => { + return; + } + } + do_something(); // $ Alert[rust/dead-code] } fn unreachable_loop() { - loop { - do_something(); - break; - do_something(); // BAD: unreachable code - } - - if cond() { - while cond() { - do_something(); - } - - while false { - do_something(); // BAD: unreachable code - } - - while true { - do_something(); - } - do_something(); // BAD: unreachable code - } - - for _ in 1..10 { - if cond() { - continue; - do_something(); // BAD: unreachable code - } - do_something(); - } - - loop { - if cond() { - return; - do_something(); // BAD: unreachable code - } - } - do_something(); // BAD: unreachable code - do_something(); - do_something(); + loop { + do_something(); + break; + do_something(); // $ Alert[rust/dead-code] + } + + if cond() { + while cond() { + do_something(); + } + + while false { + do_something(); // $ Alert[rust/dead-code] + } + + while true { + do_something(); + } + do_something(); // $ Alert[rust/dead-code] + } + + for _ in 1..10 { + if cond() { + continue; + do_something(); // $ Alert[rust/dead-code] + } + do_something(); + } + + loop { + if cond() { + return; + do_something(); // $ Alert[rust/dead-code] + } + } + do_something(); // $ Alert[rust/dead-code] + do_something(); + do_something(); } fn unreachable_paren() { - let _ = (((1))); + let _ = (1); } fn unreachable_let_1() { - if let Some(_) = maybe_get_a_number() { - do_something(); - return; - } else { - do_something(); - } - - do_something(); - - if let _ = get_a_number() { // (always succeeds) - do_something(); - return; - } else { - do_something(); // BAD: unreachable code - } - - do_something(); + if let Some(_) = maybe_get_a_number() { + do_something(); + return; + } else { + do_something(); + } + + do_something(); + + if let _ = get_a_number() { + // (always succeeds) + do_something(); + return; + } else { + do_something(); // $ Alert[rust/dead-code] + } + + do_something(); } fn unreachable_let_2() { - let Some(_) = maybe_get_a_number() else { - do_something(); - return; - }; + let Some(_) = maybe_get_a_number() else { + do_something(); + return; + }; - do_something(); + do_something(); - let _ = maybe_get_a_number() else { // (always succeeds) - do_something(); // BAD: unreachable code - return; - }; + let _ = maybe_get_a_number() else { + // (always succeeds) + do_something(); // $ Alert[rust/dead-code] + return; + }; - do_something(); + do_something(); } fn unreachable_if_2() { - if cond() { - do_something(); - return; - } else { - do_something(); - } - - do_something(); + if cond() { + do_something(); + return; + } else { + do_something(); + } + + do_something(); } fn unreachable_if_3() { - if !cond() { - do_something(); - return; - } + if !cond() { + do_something(); + return; + } - do_something(); + do_something(); } diff --git a/rust/ql/test/utils/InlineExpectationsTestQuery.ql b/rust/ql/test/utils/InlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..e5821ba4f50c --- /dev/null +++ b/rust/ql/test/utils/InlineExpectationsTestQuery.ql @@ -0,0 +1,21 @@ +/** + * @kind test-postprocess + */ + +private import rust +private import codeql.util.test.InlineExpectationsTest as T +private import internal.InlineExpectationsTestImpl +import T::TestPostProcessing +import T::TestPostProcessing::Make + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Location location) { + exists(File f, int startline, int startcolumn, int endline, int endcolumn | + location.hasLocationInfo(_, startline, startcolumn, endline, endcolumn) and + f = location.getFile() + | + result = + f.getRelativePath() + ":" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn + ) + } +} From baeffa2345268f9a463f4a6d179fb31706094050 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 29 Oct 2024 15:00:09 +0100 Subject: [PATCH 12/15] Update rust/ql/test/query-tests/unusedentities/unreachable.rs Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com> --- rust/ql/test/query-tests/unusedentities/unreachable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/ql/test/query-tests/unusedentities/unreachable.rs b/rust/ql/test/query-tests/unusedentities/unreachable.rs index 6bc064e145e4..3aecbed48866 100644 --- a/rust/ql/test/query-tests/unusedentities/unreachable.rs +++ b/rust/ql/test/query-tests/unusedentities/unreachable.rs @@ -183,7 +183,7 @@ fn unreachable_loop() { } fn unreachable_paren() { - let _ = (1); + let _ = (((1))); } fn unreachable_let_1() { From cc94c42f87874f501ebba0621fbbe2722a620931 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 29 Oct 2024 20:36:16 +0100 Subject: [PATCH 13/15] Address review comments --- .../query-tests/Security/CWE/CWE-022/semmle/tests/test.c | 2 +- .../test/query-tests/Security/CWE-094/UnsafeJsEval.swift | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c index 9185b02f0c4a..e27bfc85b8d8 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c @@ -73,7 +73,7 @@ int main(int argc, char** argv) { // $ Source=argv char buffer[1024]; read(0, buffer, 1024); // $ Source=read_output1 read(0, buffer, 1024); // $ Source=read_output2 - fopen(buffer, "wb+"); // $ Alert=read_output1 $ Alert=read_output2 + fopen(buffer, "wb+"); // $ SPURIOUS: Alert=read_output1 $ Alert=read_output2 [duplicated with both sources] } { diff --git a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift index a0b9fd0d78ce..973b28588578 100644 --- a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift +++ b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift @@ -201,17 +201,17 @@ func testSync(_ sink: @escaping (String) -> ()) { let url = URL(string: "http://example.com/") sink(localString) // GOOD: the HTML data is local - sink(try! String(contentsOf: URL(string: "http://example.com/")!)) // $ Source=source1 $ MISSING: Alert HTML contains remote input, may access local secrets - sink(try! String(contentsOf: url!)) // $ Source=source2 $ MISSING: Alert + sink(try! String(contentsOf: URL(string: "http://example.com/")!)) // $ Source=source1 + sink(try! String(contentsOf: url!)) // $ Source=source2 sink("console.log(" + localStringFragment + ")") // GOOD: the HTML data is local - sink("console.log(" + (try! String(contentsOf: url!)) + ")") // $ Source=source3 $ MISSING: Alert + sink("console.log(" + (try! String(contentsOf: url!)) + ")") // $ Source=source3 let localData = Data(localString.utf8) let remoteData = Data((try! String(contentsOf: url!)).utf8) // $ Source=source4 sink(String(decoding: localData, as: UTF8.self)) // GOOD: the data is local - sink(String(decoding: remoteData, as: UTF8.self)) // $ MISSING: Alert the data is remote + sink(String(decoding: remoteData, as: UTF8.self)) sink("console.log(" + String(Int(localStringFragment) ?? 0) + ")") // GOOD: Primitive conversion sink("console.log(" + String(Int(try! String(contentsOf: url!)) ?? 0) + ")") // GOOD: Primitive conversion From ff9811b4880c00505be8e1a8b51d6ecc4e2e3973 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Oct 2024 10:07:46 +0100 Subject: [PATCH 14/15] C#: Add tests for the inline test post-processor --- .../TestUtilities/inline-tests/InlineTests.cs | 82 +++++++++++++++++++ .../inline-tests/PathProblemQuery.expected | 23 ++++++ .../inline-tests/PathProblemQuery.qlref | 2 + .../inline-tests/ProblemQuery.expected | 9 ++ .../inline-tests/ProblemQuery.qlref | 2 + .../queries/PathProblemQuery.expected | 2 + .../inline-tests/queries/PathProblemQuery.ql | 18 ++++ .../queries/ProblemQuery.expected | 0 .../inline-tests/queries/ProblemQuery.ql | 10 +++ 9 files changed, 148 insertions(+) create mode 100644 csharp/ql/test/TestUtilities/inline-tests/InlineTests.cs create mode 100644 csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected create mode 100644 csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.qlref create mode 100644 csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.expected create mode 100644 csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.qlref create mode 100644 csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.expected create mode 100644 csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.ql create mode 100644 csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.expected create mode 100644 csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.ql diff --git a/csharp/ql/test/TestUtilities/inline-tests/InlineTests.cs b/csharp/ql/test/TestUtilities/inline-tests/InlineTests.cs new file mode 100644 index 000000000000..c30823425ca7 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/InlineTests.cs @@ -0,0 +1,82 @@ +class C +{ + void Problems() + { + // correct expectation comment, but only for `problem-query` + var x = "Alert"; // $ Alert + + // irrelevant expectation comment, will be ignored + x = "Not an alert"; // $ IrrelevantTag + + // incorrect expectation comment + x = "Also not an alert"; // $ Alert + + // missing expectation comment, but only for `problem-query` + x = "Alert"; + + // correct expectation comment + x = "Alert"; // $ Alert[problem-query] + } + + void PathProblems() + { + // correct expectation comments, but only for `path-problem-query` + var source = "Source"; // $ Source + var sink = "Sink"; // $ Sink + var x = "Alert:2:1"; // $ Alert + + // incorrect expectation comments + source = "Source"; // $ Source + sink = "Sink"; // $ Sink + x = "Not an alert:2:1"; // $ Alert + + // missing expectation comments, but only for `path-problem-query` + source = "Source"; + sink = "Sink"; + x = "Alert:2:1"; + + // correct expectation comments + source = "Source"; // $ Source[path-problem-query] + sink = "Sink"; // $ Sink[path-problem-query] + x = "Alert:2:1"; // $ Alert[path-problem-query] + + // correct expectation comments; the alert location coincides with the sink location + source = "Source"; // $ Source[path-problem-query] + x = "Alert:1:0"; // $ Alert[path-problem-query] + + // correct expectation comments; the alert location coincides with the source location + sink = "Sink"; // $ Sink[path-problem-query] + x = "Alert:0:1"; // $ Alert[path-problem-query] + + // correct expectation comments, using an identifier tag + source = "Source"; // $ Source[path-problem-query]=source1 + sink = "Sink"; // $ Sink[path-problem-query]=source1 + x = "Alert:2:1"; // $ Alert[path-problem-query]=source1 + + // incorrect expectation comment, using wrong identifier tag at the sink + source = "Source"; // $ Source[path-problem-query]=source2 + sink = "Sink"; // $ Sink[path-problem-query]=source1 + x = "Alert:2:1"; // $ Alert[path-problem-query]=source2 + + // incorrect expectation comment, using wrong identifier tag at the alert + source = "Source"; // $ Source[path-problem-query]=source3 + sink = "Sink"; // $ Sink[path-problem-query]=source3 + x = "Alert:2:1"; // $ Alert[path-problem-query]=source2 + + // correct expectation comments, using an identifier tag; the alert location coincides with the sink location + source = "Source"; // $ Source[path-problem-query]=source4 + x = "Alert:1:0"; // $ Alert[path-problem-query]=source4 + + // incorrect expectation comments, using an identifier tag; the alert location coincides with the sink location + source = "Source"; // $ Source[path-problem-query]=source5 + x = "Alert:1:0"; // $ Alert[path-problem-query]=source4 + + // correct expectation comments, using an identifier tag; the alert location coincides with the source location + sink = "Sink"; // $ Sink[path-problem-query]=sink1 + x = "Alert:0:1"; // $ Alert[path-problem-query]=sink1 + + // incorrect expectation comments, using an identifier tag; the alert location coincides with the source location + sink = "Sink"; // $ Sink[path-problem-query]=sink2 + x = "Alert:0:1"; // $ Alert[path-problem-query]=sink1 + } +} \ No newline at end of file diff --git a/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected new file mode 100644 index 000000000000..d96dde952c06 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected @@ -0,0 +1,23 @@ +#select +| InlineTests.cs:26:17:26:27 | "Alert:2:1" | InlineTests.cs:24:22:24:29 | "Source" | InlineTests.cs:25:20:25:25 | "Sink" | This is a problem | +| InlineTests.cs:36:13:36:23 | "Alert:2:1" | InlineTests.cs:34:18:34:25 | "Source" | InlineTests.cs:35:16:35:21 | "Sink" | This is a problem | +| InlineTests.cs:41:13:41:23 | "Alert:2:1" | InlineTests.cs:39:18:39:25 | "Source" | InlineTests.cs:40:16:40:21 | "Sink" | This is a problem | +| InlineTests.cs:45:13:45:23 | "Alert:1:0" | InlineTests.cs:44:18:44:25 | "Source" | InlineTests.cs:45:13:45:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:49:13:49:23 | "Alert:0:1" | InlineTests.cs:49:13:49:23 | "Alert:0:1" | InlineTests.cs:48:16:48:21 | "Sink" | This is a problem | +| InlineTests.cs:54:13:54:23 | "Alert:2:1" | InlineTests.cs:52:18:52:25 | "Source" | InlineTests.cs:53:16:53:21 | "Sink" | This is a problem | +| InlineTests.cs:59:13:59:23 | "Alert:2:1" | InlineTests.cs:57:18:57:25 | "Source" | InlineTests.cs:58:16:58:21 | "Sink" | This is a problem | +| InlineTests.cs:64:13:64:23 | "Alert:2:1" | InlineTests.cs:62:18:62:25 | "Source" | InlineTests.cs:63:16:63:21 | "Sink" | This is a problem | +| InlineTests.cs:68:13:68:23 | "Alert:1:0" | InlineTests.cs:67:18:67:25 | "Source" | InlineTests.cs:68:13:68:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:72:13:72:23 | "Alert:1:0" | InlineTests.cs:71:18:71:25 | "Source" | InlineTests.cs:72:13:72:23 | "Alert:1:0" | This is a problem | +| InlineTests.cs:76:13:76:23 | "Alert:0:1" | InlineTests.cs:76:13:76:23 | "Alert:0:1" | InlineTests.cs:75:16:75:21 | "Sink" | This is a problem | +| InlineTests.cs:80:13:80:23 | "Alert:0:1" | InlineTests.cs:80:13:80:23 | "Alert:0:1" | InlineTests.cs:79:16:79:21 | "Sink" | This is a problem | +edges +testFailures +| InlineTests.cs:6:26:6:35 | // ... | Missing result: Alert | +| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert | +| InlineTests.cs:29:28:29:38 | // ... | Missing result: Source | +| InlineTests.cs:30:24:30:32 | // ... | Missing result: Sink | +| InlineTests.cs:31:33:31:42 | // ... | Missing result: Alert | +| InlineTests.cs:34:18:34:25 | "Source" | Unexpected result: Source | +| InlineTests.cs:35:16:35:21 | "Sink" | Unexpected result: Sink | +| InlineTests.cs:36:13:36:23 | InlineTests.cs:34:18:34:25 | Unexpected result: Alert | diff --git a/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.qlref b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.qlref new file mode 100644 index 000000000000..cbc554598f33 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.qlref @@ -0,0 +1,2 @@ +query: TestUtilities/inline-tests/queries/PathProblemQuery.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.expected b/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.expected new file mode 100644 index 000000000000..88fe5019fa2a --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.expected @@ -0,0 +1,9 @@ +#select +| InlineTests.cs:6:17:6:23 | "Alert" | This is a problem | +| InlineTests.cs:15:13:15:19 | "Alert" | This is a problem | +| InlineTests.cs:18:13:18:19 | "Alert" | This is a problem | +testFailures +| InlineTests.cs:12:34:12:43 | // ... | Missing result: Alert | +| InlineTests.cs:15:13:15:19 | This is a problem | Unexpected result: Alert | +| InlineTests.cs:26:30:26:39 | // ... | Missing result: Alert | +| InlineTests.cs:31:33:31:42 | // ... | Missing result: Alert | diff --git a/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.qlref b/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.qlref new file mode 100644 index 000000000000..d5afc6a690f2 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/ProblemQuery.qlref @@ -0,0 +1,2 @@ +query: TestUtilities/inline-tests/queries/ProblemQuery.ql +postprocess: TestUtilities/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.expected b/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.expected new file mode 100644 index 000000000000..5c153698e9e8 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.expected @@ -0,0 +1,2 @@ +edges +#select diff --git a/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.ql b/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.ql new file mode 100644 index 000000000000..748fa47c5c21 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/queries/PathProblemQuery.ql @@ -0,0 +1,18 @@ +/** + * @kind path-problem + * @id path-problem-query + */ + +import csharp + +query predicate edges(StringLiteral sl1, StringLiteral sl2) { none() } + +from StringLiteral alert, StringLiteral source, StringLiteral sink +where + exists(string regexp, int sourceOffset, int sinkOffset | regexp = "Alert:([0-9]+):([0-9]+)" | + sourceOffset = alert.getValue().regexpCapture(regexp, 1).toInt() and + sinkOffset = alert.getValue().regexpCapture(regexp, 2).toInt() and + source.getLocation().getStartLine() = alert.getLocation().getStartLine() - sourceOffset and + sink.getLocation().getStartLine() = alert.getLocation().getStartLine() - sinkOffset + ) +select alert, source, sink, "This is a problem" diff --git a/csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.expected b/csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.ql b/csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.ql new file mode 100644 index 000000000000..8a1f057f0bc9 --- /dev/null +++ b/csharp/ql/test/TestUtilities/inline-tests/queries/ProblemQuery.ql @@ -0,0 +1,10 @@ +/** + * @kind problem + * @id problem-query + */ + +import csharp + +from StringLiteral sl +where sl.getValue() = "Alert" +select sl, "This is a problem" From 495c92df38a53776d23bd8bae6844eb4f4289e24 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Oct 2024 10:09:00 +0100 Subject: [PATCH 15/15] Shared: Also take query ID into account in `PathProblemSourceTestInput` --- .../inline-tests/PathProblemQuery.expected | 8 ++ .../util/test/InlineExpectationsTest.qll | 118 +++++++++--------- 2 files changed, 69 insertions(+), 57 deletions(-) diff --git a/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected index d96dde952c06..76674dc15695 100644 --- a/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected +++ b/csharp/ql/test/TestUtilities/inline-tests/PathProblemQuery.expected @@ -21,3 +21,11 @@ testFailures | InlineTests.cs:34:18:34:25 | "Source" | Unexpected result: Source | | InlineTests.cs:35:16:35:21 | "Sink" | Unexpected result: Sink | | InlineTests.cs:36:13:36:23 | InlineTests.cs:34:18:34:25 | Unexpected result: Alert | +| InlineTests.cs:58:16:58:21 | "Sink" | Unexpected result: Sink=source2 | +| InlineTests.cs:58:24:58:60 | // ... | Missing result: Sink[path-problem-query]=source1 | +| InlineTests.cs:64:13:64:23 | InlineTests.cs:62:18:62:25 | Unexpected result: Alert=source3 | +| InlineTests.cs:64:26:64:63 | // ... | Missing result: Alert[path-problem-query]=source2 | +| InlineTests.cs:72:13:72:23 | "Alert:1:0" | Unexpected result: Alert=source5 | +| InlineTests.cs:72:26:72:63 | // ... | Missing result: Alert[path-problem-query]=source4 | +| InlineTests.cs:79:16:79:21 | "Sink" | Unexpected result: Sink=sink1 | +| InlineTests.cs:79:24:79:58 | // ... | Missing result: Sink[path-problem-query]=sink2 | diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 8bf8a7db95b6..bd51ad1c2bd8 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -717,13 +717,44 @@ module TestPostProcessing { ) } + private string getTagRegex() { + exists(string sourceSinkTags | + ( + getQueryKind() = "problem" + or + not exists(getSourceTag(_)) and + not exists(getSinkTag(_)) + ) and + sourceSinkTags = "" + or + sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_) + | + result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?" + ) + } + /** * A configuration for matching `// $ Source=foo` comments against actual * path-problem sources. + * + * Whenever a source is tagged with a value, like `foo`, we will use that + * to define the expected tags at the sink and the alert. */ private module PathProblemSourceTestInput implements TestSig { string getARelevantTag() { result = getSourceTag(_) } + bindingset[expectedTag, actualTag] + predicate tagMatches(string expectedTag, string actualTag) { + actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and + ( + // expected tag is annotated with a query ID + getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3) + or + // expected tag is not annotated with a query ID + not exists(expectedTag.regexpCapture(getTagRegex(), 3)) + ) + } + bindingset[expectedValue, actualValue] predicate valueMatches(string expectedValue, string actualValue) { exists(expectedValue) and @@ -754,28 +785,7 @@ module TestPostProcessing { bindingset[result] string getARelevantTag() { any() } - private string getTagRegex() { - exists(string sourceSinkTags | - getQueryKind() = "problem" and - sourceSinkTags = "" - or - sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_) - | - result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?" - ) - } - - bindingset[expectedTag, actualTag] - predicate tagMatches(string expectedTag, string actualTag) { - actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and - ( - // expected tag is annotated with a query ID - getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3) - or - // expected tag is not annotated with a query ID - not exists(expectedTag.regexpCapture(getTagRegex(), 3)) - ) - } + predicate tagMatches = PathProblemSourceTestInput::tagMatches/2; bindingset[expectedTag] predicate tagIsOptional(string expectedTag) { @@ -789,31 +799,10 @@ module TestPostProcessing { ) } - bindingset[expectedValue, actualValue] - predicate valueMatches(string expectedValue, string actualValue) { - expectedValue = actualValue - or - actualValue = "" - } - private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5; - /** - * Gets the expected sink value for result row `row`. This value must - * match the value at the corresponding path-problem source (if it is - * present). - */ - private string getSinkValue(int row) { - exists(Input::Location location, string element, string tag, string val | - hasPathProblemSource(row, location, element, tag, val) and - result = - PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false) - .getValue() - ) - } - private predicate hasPathProblemSink( - int row, Input::Location location, string element, string tag, string value + int row, Input::Location location, string element, string tag ) { getQueryKind() = "path-problem" and exists(string loc | @@ -821,32 +810,47 @@ module TestPostProcessing { queryResults("#select", row, 5, element) and tag = getSinkTag(row) and Input2::getRelativeUrl(location) = loc - | - not exists(getSinkValue(row)) and value = "" - or - value = getSinkValue(row) ) } - private predicate hasAlert(Input::Location location, string element, string tag, string value) { + private predicate hasAlert(int row, Input::Location location, string element, string tag) { getQueryKind() = ["problem", "path-problem"] and - exists(int row, string loc | + exists(string loc | queryResults("#select", row, 0, loc) and queryResults("#select", row, 2, element) and tag = "Alert" and - value = "" and Input2::getRelativeUrl(location) = loc and not hasPathProblemSource(row, location, _, _, _) and - not hasPathProblemSink(row, location, _, _, _) + not hasPathProblemSink(row, location, _, _) + ) + } + + /** + * Gets the expected value for result row `row`, if any. This value must + * match the value at the corresponding path-problem source (if it is + * present). + */ + private string getValue(int row) { + exists(Input::Location location, string element, string tag, string val | + hasPathProblemSource(row, location, element, tag, val) and + result = + PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false) + .getValue() ) } predicate hasActualResult(Input::Location location, string element, string tag, string value) { - hasPathProblemSource(_, location, element, tag, value) - or - hasPathProblemSink(_, location, element, tag, value) - or - hasAlert(location, element, tag, value) + exists(int row | + hasPathProblemSource(row, location, element, tag, _) + or + hasPathProblemSink(row, location, element, tag) + or + hasAlert(row, location, element, tag) + | + not exists(getValue(row)) and value = "" + or + value = getValue(row) + ) } }