Skip to content

Commit 50c247a

Browse files
hvitvedhvitved
committed
wip
1 parent 9a1df79 commit 50c247a

File tree

7 files changed

+241
-66
lines changed

7 files changed

+241
-66
lines changed

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/Index.cshtml.g.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ public class Pages_Index : global::Microsoft.AspNetCore.Mvc.RazorPages.Page
3030
#line 3 "Index.cshtml"
3131

3232
ViewData["Title"] = "ASP.NET Core";
33-
var message = Request.Query["m"];
33+
var message = Request.Query["m"]; // $ Source=message
3434

3535
#line default
3636
#line hidden
3737
#nullable disable
3838
WriteLiteral("<div class=\"cli\">\n <div class=\"cli-example\"> \n");
3939
#nullable restore
4040
#line 14 "Index.cshtml"
41-
Write(Html.Raw(message)); // BAD
41+
Write(Html.Raw(message)); // $ Alert=message
4242

4343
#line default
4444
#line hidden

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected‎

Lines changed: 59 additions & 35 deletions
Large diffs are not rendered by default.

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.qlref‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
query: Security Features/CWE-079/XSS.ql
2-
postprocess: TestUtilities/PrettyPrintModels.ql
2+
postprocess:
3+
- TestUtilities/PrettyPrintModels.ql
4+
- TestUtilities/InlineExpectationsTestQuery.ql

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNet.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ public override void Execute()
1616
{
1717
Layout = "~/_SiteLayout.cshtml";
1818
Page.Title = "Contact";
19-
var sayHi = Request.QueryString["sayHi"];
19+
var sayHi = Request.QueryString["sayHi"]; // $ Source=sayHi
2020
if (sayHi.IsEmpty())
2121
{
2222
WriteLiteral("<script>alert(\"XSS via WriteLiteral\")</script>"); // GOOD: hard-coded, not user input
2323
}
2424
else
2525
{
26-
WriteLiteral(sayHi); // BAD: user input flows to HTML unencoded
26+
WriteLiteral(sayHi); // $ Alert=sayHi
2727
WriteLiteral(HttpUtility.HtmlEncode(sayHi)); // Good: user input is encoded before it flows to HTML
2828
}
2929

@@ -33,15 +33,16 @@ public override void Execute()
3333
}
3434
else
3535
{
36-
WriteLiteralTo(Output, sayHi); // BAD: user input flows to HTML unencoded
36+
WriteLiteralTo(Output, sayHi); // $ Alert=sayHi
3737
WriteLiteralTo(Output, Html.Encode(sayHi)); // Good: user input is encoded before it flows to HTML
3838
}
3939

4040
BeginContext("~/Views/Home/Contact.cshtml", 288, 32, false);
4141

4242
Write(Html.Raw("<script>alert(\"XSS via Html.Raw()\")</script>")); // GOOD: hard-coded, not user input
43-
Write(Html.Raw(Request.QueryString["sayHi"])); // BAD: user input flows to HTML unencoded
44-
Write(Html.Raw(HttpContext.Current.Server.HtmlEncode(Request.QueryString["sayHi"]))); // Good: user input is encoded before it flows to HTML
43+
var sayHi2 = Request.QueryString["sayHi"]; // $ Source=sayHi2
44+
Write(Html.Raw(sayHi2)); // $ Alert=sayHi2
45+
Write(Html.Raw(HttpContext.Current.Server.HtmlEncode(sayHi2))); // Good: user input is encoded before it flows to HTML
4546
EndContext("~/Views/Home/Contact.cshtml", 288, 32, false);
4647
}
4748
}

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSSAspNetCore.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ public IActionResult Index()
1818
{
1919
// BAD: flow of content type to.
2020
var v = new ViewResult();
21-
v.ViewData["BadData"] = new HtmlString(Request.Query["Bad data"]);
21+
var source = Request.Query["Bad data"]; // $ Source=req1
22+
v.ViewData["BadData"] = new HtmlString(source); // $ Alert=req1
2223

2324
StringValues vOut;
2425
Request.Query.TryGetValue("Foo", out vOut);
@@ -37,39 +38,44 @@ public IActionResult Index()
3738

3839
[HttpPost("Test")]
3940
[ValidateAntiForgeryToken]
40-
public IActionResult Submit([FromQuery] string foo)
41+
public IActionResult Submit([FromQuery] string foo) // $ Source=foo
4142
{
4243
var view = new ViewResult();
4344
//BAD: flow of submitted value to view in HtmlString.
44-
view.ViewData["FOO"] = new HtmlString(foo);
45+
view.ViewData["FOO"] = new HtmlString(foo); // $ Alert=foo
4546
return view;
4647
}
4748

4849
public IActionResult IndexToModel()
4950
{
5051
//BAD: flow of submitted value to view in HtmlString.
51-
HtmlString v = new HtmlString(Request.QueryString.Value);
52+
var req2 = Request.QueryString.Value; // $ Source=req2
53+
HtmlString v = new HtmlString(req2); // $ Alert=req2
5254
return View(new HomeViewModel() { Message = "Message from Index", Description = v });
5355
}
5456

5557
public IActionResult About()
5658
{
5759
//BAD: flow of submitted value to view in HtmlString.
58-
HtmlString v = new HtmlString(Request.Query["Foo"].ToString());
60+
var req3 = Request.Query["Foo"].ToString(); // $ Source=req3
61+
HtmlString v = new HtmlString(req3); // $ Alert=req3
5962

6063
//BAD: flow of submitted value to view in HtmlString.
61-
HtmlString v1 = new HtmlString(Request.Query["Foo"][0]);
64+
var req4 = Request.Query["Foo"][0]; // $ Source=req4
65+
HtmlString v1 = new HtmlString(req4); // $ Alert=req4
6266

6367
return View(new HomeViewModel() { Message = "Message from About", Description = v });
6468
}
6569

6670
public IActionResult Contact()
6771
{
6872
//BAD: flow of user content type to view in HtmlString.
69-
HtmlString v = new HtmlString(Request.ContentType);
73+
var ct = Request.ContentType; // $ Source=ct
74+
HtmlString v = new HtmlString(ct); // $ Alert=ct
7075

7176
//BAD: flow of headers to view in HtmlString.
72-
HtmlString v1 = new HtmlString(value: Request.Headers["Foo"]);
77+
var header = Request.Headers["Foo"]; // $ Source=header
78+
HtmlString v1 = new HtmlString(value: header); // $ Alert=header
7379

7480
return View(new HomeViewModel() { Message = "Message from Contact", Description = v });
7581
}

‎go/ql/test/library-tests/semmle/go/frameworks/Gin/TaintedPath.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,9 @@ nodes
2121
| Gin.go:27:20:27:27 | filepath | semmle.label | filepath |
2222
| Gin.go:29:32:29:39 | filepath | semmle.label | filepath |
2323
subpaths
24+
testFailures
25+
| Gin.go:24:15:24:33 | call to Query | Unexpected result: Source |
26+
| Gin.go:25:10:25:17 | filepath | Unexpected result: Sink |
27+
| Gin.go:26:39:26:46 | filepath | Unexpected result: Sink |
28+
| Gin.go:27:20:27:27 | filepath | Unexpected result: Sink |
29+
| Gin.go:29:32:29:39 | filepath | Unexpected result: Sink |

‎shared/util/codeql/util/test/InlineExpectationsTest.qll‎

Lines changed: 151 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,16 @@ module Make<InlineExpectationsTestSig Impl> {
148148
bindingset[expectedTag]
149149
default predicate tagIsOptional(string expectedTag) { none() }
150150

151+
/**
152+
* Holds if expected tag `expectedTag` matches actual tag `actualTag`.
153+
*
154+
* This is normally defined as `expectedTag = actualTag`.
155+
*/
156+
bindingset[expectedValue, actualValue]
157+
default predicate valueMatches(string expectedValue, string actualValue) {
158+
expectedValue = actualValue
159+
}
160+
151161
/**
152162
* Returns the actual results of the query that is being tested. Each result consist of the
153163
* following values:
@@ -324,7 +334,7 @@ module Make<InlineExpectationsTestSig Impl> {
324334
predicate matchesActualResult(ActualTestResult actualResult) {
325335
onSameLine(pragma[only_bind_into](this), actualResult) and
326336
TestImpl::tagMatches(this.getTag(), actualResult.getTag()) and
327-
this.getValue() = actualResult.getValue()
337+
TestImpl::valueMatches(this.getValue(), actualResult.getValue())
328338
}
329339

330340
predicate isOptional() { TestImpl::tagIsOptional(tag) }
@@ -351,6 +361,15 @@ module Make<InlineExpectationsTestSig Impl> {
351361
string getExpectation() { result = expectation }
352362
}
353363

364+
ValidTestExpectation getAMatchingExpectation(
365+
Impl::Location location, string element, string tag, string val, boolean optional
366+
) {
367+
exists(ActualTestResult actualResult |
368+
result.matchesActualResult(actualResult) and
369+
actualResult = TActualResult(location, element, tag, val, optional)
370+
)
371+
}
372+
354373
query predicate testFailures(FailureLocatable element, string message) {
355374
hasFailureMessage(element, message)
356375
}
@@ -622,6 +641,8 @@ module TestPostProcessing {
622641

623642
private string getQueryId() { queryMetadata("id", result) }
624643

644+
private string getQueryKind() { queryMetadata("kind", result) }
645+
625646
signature module InputSig<InlineExpectationsTestSig Input> {
626647
string getRelativeUrl(Input::Location location);
627648
}
@@ -630,43 +651,158 @@ module TestPostProcessing {
630651
private import InlineExpectationsTest as InlineExpectationsTest
631652
private import InlineExpectationsTest::Make<Input>
632653

654+
/**
655+
* Gets the tag to be used for the path-problem source at result row `row`.
656+
*
657+
* This is either `Source` or `Alert`, depending on whether the location
658+
* of the source matches the location of the alert.
659+
*/
660+
private string getSourceTag(int row) {
661+
getQueryKind() = "path-problem" and
662+
exists(string loc | queryResults("#select", row, 2, loc) |
663+
if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Source"
664+
)
665+
}
666+
667+
/**
668+
* Gets the tag to be used for the path-problem sink at result row `row`.
669+
*
670+
* This is either `Sink` or `Alert`, depending on whether the location
671+
* of the sink matches the location of the alert.
672+
*/
673+
private string getSinkTag(int row) {
674+
getQueryKind() = "path-problem" and
675+
exists(string loc | queryResults("#select", row, 4, loc) |
676+
if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Sink"
677+
)
678+
}
679+
680+
/**
681+
* A configuration for matching `// $ Source=foo` comments against actual
682+
* path-problem sources.
683+
*/
684+
private module PathProblemSourceTestInput implements TestSig {
685+
string getARelevantTag() { result = getSourceTag(_) }
686+
687+
bindingset[expectedValue, actualValue]
688+
predicate valueMatches(string expectedValue, string actualValue) {
689+
exists(expectedValue) and
690+
actualValue = ""
691+
}
692+
693+
additional predicate hasPathProblemSource(
694+
int row, Input::Location location, string element, string tag, string value
695+
) {
696+
getQueryKind() = "path-problem" and
697+
exists(string loc |
698+
queryResults("#select", row, 2, loc) and
699+
queryResults("#select", row, 3, element) and
700+
tag = getSourceTag(row) and
701+
value = "" and
702+
Input2::getRelativeUrl(location) = loc
703+
)
704+
}
705+
706+
predicate hasActualResult(Input::Location location, string element, string tag, string value) {
707+
hasPathProblemSource(_, location, element, tag, value)
708+
}
709+
}
710+
711+
private module PathProblemSourceTest = MakeTest<PathProblemSourceTestInput>;
712+
633713
private module TestInput implements TestSig {
634714
bindingset[result]
635715
string getARelevantTag() { any() }
636716

717+
private string getTagRegex() {
718+
exists(string sourceSinkTags |
719+
getQueryKind() = "problem" and
720+
sourceSinkTags = ""
721+
or
722+
sourceSinkTags = "|" + getSourceTag(_) + "|" + getSinkTag(_)
723+
|
724+
result = "(Alert" + sourceSinkTags + ")(\\[(.*)\\])?"
725+
)
726+
}
727+
637728
bindingset[expectedTag, actualTag]
638729
predicate tagMatches(string expectedTag, string actualTag) {
730+
actualTag = expectedTag.regexpCapture(getTagRegex(), 1) and
639731
(
640-
expectedTag = "Alert"
732+
getQueryId() = expectedTag.regexpCapture(getTagRegex(), 3)
641733
or
642-
exists(string alertTagRegex |
643-
alertTagRegex = "Alert\\[(.*)\\]" and
644-
getQueryId() = expectedTag.regexpCapture(alertTagRegex, 1)
645-
)
646-
) and
647-
exists(actualTag)
734+
not exists(expectedTag.regexpCapture(getTagRegex(), 3))
735+
)
648736
}
649737

650738
bindingset[expectedTag]
651739
predicate tagIsOptional(string expectedTag) {
652-
not expectedTag.regexpMatch("Alert(\\[.*\\])?")
740+
not expectedTag.regexpMatch(getTagRegex())
653741
or
654-
exists(string alertTagRegex, string queryId |
655-
alertTagRegex = "Alert\\[(.*)\\]" and
656-
queryId = expectedTag.regexpCapture(alertTagRegex, 1) and
657-
not queryId = getQueryId()
742+
exists(string queryId |
743+
queryId = expectedTag.regexpCapture(getTagRegex(), 3) and
744+
queryId != getQueryId()
658745
)
659746
}
660747

661-
predicate hasActualResult(Input::Location location, string element, string tag, string value) {
748+
bindingset[expectedValue, actualValue]
749+
predicate valueMatches(string expectedValue, string actualValue) {
750+
expectedValue = actualValue
751+
or
752+
actualValue = ""
753+
}
754+
755+
private predicate hasPathProblemSource = PathProblemSourceTestInput::hasPathProblemSource/5;
756+
757+
/**
758+
* Gets the expected sink value for result row `row`. This value must
759+
* match the value at the corresponding path-problem source.
760+
*/
761+
private string getSinkValue(int row) {
762+
exists(Input::Location location, string element, string tag, string val |
763+
hasPathProblemSource(row, location, element, tag, val) and
764+
result =
765+
PathProblemSourceTest::getAMatchingExpectation(location, element, tag, val, false)
766+
.getValue()
767+
)
768+
}
769+
770+
private predicate hasPathProblemSink(
771+
int row, Input::Location location, string element, string tag, string value
772+
) {
773+
getQueryKind() = "path-problem" and
774+
exists(string loc |
775+
queryResults("#select", row, 4, loc) and
776+
queryResults("#select", row, 5, element) and
777+
tag = getSinkTag(row) and
778+
Input2::getRelativeUrl(location) = loc
779+
|
780+
not exists(getSinkValue(row)) and value = ""
781+
or
782+
value = getSinkValue(row)
783+
)
784+
}
785+
786+
private predicate hasAlert(Input::Location location, string element, string tag, string value) {
787+
getQueryKind() = ["problem", "path-problem"] and
662788
exists(int row, string loc |
663789
queryResults("#select", row, 0, loc) and
664790
queryResults("#select", row, 2, element) and
665791
tag = "Alert" and
666792
value = "" and
667-
Input2::getRelativeUrl(location) = loc
793+
Input2::getRelativeUrl(location) = loc and
794+
not hasPathProblemSource(row, location, _, _, _) and
795+
not hasPathProblemSink(row, location, _, _, _)
668796
)
669797
}
798+
799+
predicate hasActualResult(Input::Location location, string element, string tag, string value) {
800+
hasAlert(location, element, tag, value)
801+
or
802+
hasPathProblemSource(_, location, element, tag, value)
803+
or
804+
hasPathProblemSink(_, location, element, tag, value)
805+
}
670806
}
671807

672808
private module Test = MakeTest<TestInput>;

0 commit comments

Comments
 (0)