Skip to content

Commit 5016113

Browse files
committed
C#: Add a string.Format sanitizer to url redirect and update expected test output.
1 parent 884f3f1 commit 5016113

File tree

3 files changed

+16
-7
lines changed

3 files changed

+16
-7
lines changed

csharp/ql/lib/semmle/code/csharp/frameworks/Format.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class ValidFormatString extends StringLiteral {
134134
result = this.getValue().regexpFind(getValidFormatTokenRegex(), _, outPosition)
135135
}
136136

137-
/**Gets the insert number at the given position in the string. */
137+
/** Gets the insert number at the given position in the string. */
138138
int getInsert(int position) {
139139
result = this.getToken(position).regexpCapture(getFormatInsertRegex(), 1).toInt()
140140
}

csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import csharp
66
private import semmle.code.csharp.security.dataflow.flowsources.Remote
77
private import semmle.code.csharp.controlflow.Guards
8+
private import semmle.code.csharp.frameworks.Format
89
private import semmle.code.csharp.frameworks.system.Web
910
private import semmle.code.csharp.frameworks.system.web.Mvc
1011
private import semmle.code.csharp.security.Sanitizers
@@ -175,6 +176,20 @@ private class InterpolationSanitizer extends Sanitizer {
175176
}
176177
}
177178

179+
/**
180+
* A call to `string.Format`, where the format expression (before any inserts)
181+
* contains the character "?".
182+
*
183+
* This is considered a sanitizer by the same reasoning as `ConcatenationSanitizer`.
184+
*/
185+
private class StringFormatSanitizer extends Sanitizer {
186+
StringFormatSanitizer() {
187+
exists(FormatCall c, Expr e | c = this.getExpr() and e = c.getFormatExpr() |
188+
e.(StringLiteral).getValue().splitAt("{0}", 0).matches("%?%")
189+
)
190+
}
191+
}
192+
178193
/** A call to an URL encoder. */
179194
class UrlEncodeSanitizer extends Sanitizer {
180195
UrlEncodeSanitizer() { this.getExpr() instanceof UrlSanitizedExpr }

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@ edges
33
| UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:23:22:23:52 | access to indexer : String |
44
| UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:48:29:48:31 | access to local variable url |
55
| UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:64:31:64:52 | $"..." |
6-
| UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:67:66:67:68 | access to local variable url : String |
76
| UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:70:66:70:68 | access to local variable url : String |
87
| UrlRedirect.cs:23:22:23:52 | access to indexer : String | UrlRedirect.cs:48:29:48:31 | access to local variable url |
98
| UrlRedirect.cs:23:22:23:52 | access to indexer : String | UrlRedirect.cs:64:31:64:52 | $"..." |
10-
| UrlRedirect.cs:23:22:23:52 | access to indexer : String | UrlRedirect.cs:67:66:67:68 | access to local variable url : String |
119
| UrlRedirect.cs:23:22:23:52 | access to indexer : String | UrlRedirect.cs:70:66:70:68 | access to local variable url : String |
1210
| UrlRedirect.cs:38:44:38:66 | access to property QueryString : NameValueCollection | UrlRedirect.cs:38:44:38:74 | access to indexer |
1311
| UrlRedirect.cs:39:47:39:69 | access to property QueryString : NameValueCollection | UrlRedirect.cs:39:47:39:77 | access to indexer |
14-
| UrlRedirect.cs:67:66:67:68 | access to local variable url : String | UrlRedirect.cs:67:31:67:69 | call to method Format |
1512
| UrlRedirect.cs:70:66:70:68 | access to local variable url : String | UrlRedirect.cs:70:31:70:69 | call to method Format |
1613
| UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:16:22:16:26 | access to parameter value |
1714
| UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:19:44:19:48 | call to operator implicit conversion |
@@ -35,8 +32,6 @@ nodes
3532
| UrlRedirect.cs:39:47:39:77 | access to indexer | semmle.label | access to indexer |
3633
| UrlRedirect.cs:48:29:48:31 | access to local variable url | semmle.label | access to local variable url |
3734
| UrlRedirect.cs:64:31:64:52 | $"..." | semmle.label | $"..." |
38-
| UrlRedirect.cs:67:31:67:69 | call to method Format | semmle.label | call to method Format |
39-
| UrlRedirect.cs:67:66:67:68 | access to local variable url : String | semmle.label | access to local variable url : String |
4035
| UrlRedirect.cs:70:31:70:69 | call to method Format | semmle.label | call to method Format |
4136
| UrlRedirect.cs:70:66:70:68 | access to local variable url : String | semmle.label | access to local variable url : String |
4237
| UrlRedirectCore.cs:13:44:13:48 | value : String | semmle.label | value : String |
@@ -59,7 +54,6 @@ subpaths
5954
| 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 |
6055
| 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 |
6156
| 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 |
62-
| UrlRedirect.cs:67:31:67:69 | call to method Format | UrlRedirect.cs:23:22:23:44 | access to property QueryString : NameValueCollection | UrlRedirect.cs:67:31:67:69 | call to method Format | Untrusted URL redirection due to $@. | UrlRedirect.cs:23:22:23:44 | access to property QueryString | user-provided value |
6357
| 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 |
6458
| UrlRedirectCore.cs:16:22:16:26 | access to parameter value | UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:16:22:16:26 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirectCore.cs:13:44:13:48 | value | user-provided value |
6559
| UrlRedirectCore.cs:19:44:19:48 | call to operator implicit conversion | UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:19:44:19:48 | call to operator implicit conversion | Untrusted URL redirection due to $@. | UrlRedirectCore.cs:13:44:13:48 | value | user-provided value |

0 commit comments

Comments
 (0)