Skip to content

Commit 2c161e6

Browse files
authored
Merge pull request #22180 from michaelnebel/csharp/rawurl
C#: Remove the RawUrl sanitizer.
2 parents 05734dc + ae06b77 commit 2c161e6

4 files changed

Lines changed: 8 additions & 8 deletions

File tree

csharp/ql/lib/ext/System.Web.model.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
11
extensions:
2-
- addsTo:
3-
pack: codeql/csharp-all
4-
extensible: barrierModel
5-
data:
6-
# The RawUrl property is considered to be safe for URL redirects
7-
- ["System.Web", "HttpRequest", False, "get_RawUrl", "()", "", "ReturnValue", "url-redirection", "manual"]
82
- addsTo:
93
pack: codeql/csharp-all
104
extensible: sinkModel
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* `System.Web.HttpRequest.RawUrl` is no longer treated as a sanitizer for `cs/web/unvalidated-url-redirection`, since it contains the un-normalized request line. This may lead to more results.

csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect/UrlRedirect.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ public void ProcessRequest(HttpContext ctx)
3838
ctx.Response.AddHeader("Location", ctx.Request.QueryString["page"]); // $ Alert=r3 Alert=r3
3939
ctx.Response.AppendHeader("Location", ctx.Request.QueryString["page"]); // $ Alert=r4 Alert=r4
4040

41-
// GOOD: Redirecting to the RawUrl only reloads the current Url
42-
ctx.Response.Redirect(ctx.Request.RawUrl);
41+
// BAD: The RawUrl contains the un-normalized request line.
42+
ctx.Response.Redirect(ctx.Request.RawUrl); // $ Alert
4343

4444
// GOOD: The attacker can only control the parameters, not the location
4545
ctx.Response.Redirect("foo.asp?param=" + url);

csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect/UrlRedirect.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
| UrlRedirect.cs:13:31:13:61 | access to indexer | UrlRedirect.cs:13:31:13:53 | access to property QueryString : NameValueCollection | UrlRedirect.cs:13:31:13:61 | access to indexer | Untrusted URL redirection due to $@. | UrlRedirect.cs:13:31:13:53 | access to property QueryString | user-provided value |
44
| UrlRedirect.cs:38:44:38:74 | access to indexer | UrlRedirect.cs:38:44:38:66 | access to property QueryString : NameValueCollection | UrlRedirect.cs:38:44:38:74 | access to indexer | Untrusted URL redirection due to $@. | UrlRedirect.cs:38:44:38:66 | access to property QueryString | user-provided value |
55
| UrlRedirect.cs:39:47:39:77 | access to indexer | UrlRedirect.cs:39:47:39:69 | access to property QueryString : NameValueCollection | UrlRedirect.cs:39:47:39:77 | access to indexer | Untrusted URL redirection due to $@. | UrlRedirect.cs:39:47:39:69 | access to property QueryString | user-provided value |
6+
| UrlRedirect.cs:42:31:42:48 | access to property RawUrl | UrlRedirect.cs:42:31:42:48 | access to property RawUrl | UrlRedirect.cs:42:31:42:48 | access to property RawUrl | Untrusted URL redirection due to $@. | UrlRedirect.cs:42:31:42:48 | access to property RawUrl | user-provided value |
67
| UrlRedirect.cs:48:29:48:31 | access to local variable url | UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:48:29:48:31 | access to local variable url | Untrusted URL redirection due to $@. | UrlRedirect.cs:23:22:23:44 | access to property QueryString | user-provided value |
78
| UrlRedirect.cs:64:31:64:52 | $"..." | UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:64:31:64:52 | $"..." | Untrusted URL redirection due to $@. | UrlRedirect.cs:23:22:23:44 | access to property QueryString | user-provided value |
89
| UrlRedirect.cs:70:31:70:69 | call to method Format | UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:70:31:70:69 | call to method Format | Untrusted URL redirection due to $@. | UrlRedirect.cs:23:22:23:44 | access to property QueryString | user-provided value |
@@ -66,6 +67,7 @@ nodes
6667
| UrlRedirect.cs:38:44:38:74 | access to indexer | semmle.label | access to indexer |
6768
| UrlRedirect.cs:39:47:39:69 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
6869
| UrlRedirect.cs:39:47:39:77 | access to indexer | semmle.label | access to indexer |
70+
| UrlRedirect.cs:42:31:42:48 | access to property RawUrl | semmle.label | access to property RawUrl |
6971
| UrlRedirect.cs:48:29:48:31 | access to local variable url | semmle.label | access to local variable url |
7072
| UrlRedirect.cs:64:31:64:52 | $"..." | semmle.label | $"..." |
7173
| UrlRedirect.cs:70:31:70:69 | call to method Format | semmle.label | call to method Format |

0 commit comments

Comments
 (0)