Skip to content

Commit b5104ae

Browse files
asgerfasgerf
committed
JS: Add StartsWith sanitizer
1 parent 4c06eb8 commit b5104ae

File tree

7 files changed

+30
-27
lines changed

7 files changed

+30
-27
lines changed

‎javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,9 @@ module ClientSideUrlRedirect {
5050
g instanceof DocumentUrl and
5151
succ.(DataFlow::PropRead).accesses(pred, "href")
5252
}
53+
54+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
55+
guard instanceof HostnameSanitizerGuard
56+
}
5357
}
5458
}

‎javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import javascript
88
import semmle.javascript.security.dataflow.RemoteFlowSources
9-
import UrlConcatenation
9+
private import UrlConcatenation
1010

1111
module ClientSideUrlRedirect {
1212
private import Xss::DomBasedXss as DomBasedXss

‎javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ module ServerSideUrlRedirect {
3434
}
3535

3636
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
37-
guard instanceof LocalUrlSanitizingGuard
37+
guard instanceof LocalUrlSanitizingGuard or
38+
guard instanceof HostnameSanitizerGuard
3839
}
3940
}
4041
}

‎javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirectCustomizations.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import javascript
88
import RemoteFlowSources
9-
import UrlConcatenation
9+
private import UrlConcatenation
1010

1111
module ServerSideUrlRedirect {
1212
/**

‎javascript/ql/src/semmle/javascript/security/dataflow/UrlConcatenation.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,17 @@ predicate hostnameSanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sin
9696
hasHostnameSanitizingSubstring(StringConcatenation::getOperand(operator, [0 .. n - 1]))
9797
)
9898
}
99+
100+
/**
101+
* A check that sanitizes the hostname of a URL.
102+
*/
103+
class HostnameSanitizerGuard extends TaintTracking::SanitizerGuardNode, StringOps::StartsWith {
104+
HostnameSanitizerGuard() {
105+
hasHostnameSanitizingSubstring(getSubstring())
106+
}
107+
108+
override predicate sanitizes(boolean outcome, Expr e) {
109+
outcome = getPolarity() and
110+
e = getBaseString().asExpr()
111+
}
112+
}

‎javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,6 @@ nodes
44
| sanitizer.js:2:15:2:25 | window.name |
55
| sanitizer.js:4:27:4:29 | url |
66
| sanitizer.js:4:27:4:29 | url |
7-
| sanitizer.js:7:27:7:29 | url |
8-
| sanitizer.js:7:27:7:29 | url |
9-
| sanitizer.js:10:27:10:29 | url |
10-
| sanitizer.js:10:27:10:29 | url |
11-
| sanitizer.js:13:27:13:29 | url |
12-
| sanitizer.js:13:27:13:29 | url |
137
| sanitizer.js:16:27:16:29 | url |
148
| sanitizer.js:16:27:16:29 | url |
159
| sanitizer.js:19:27:19:29 | url |
@@ -22,8 +16,6 @@ nodes
2216
| sanitizer.js:28:27:28:29 | url |
2317
| sanitizer.js:31:27:31:29 | url |
2418
| sanitizer.js:31:27:31:29 | url |
25-
| sanitizer.js:34:27:34:29 | url |
26-
| sanitizer.js:34:27:34:29 | url |
2719
| sanitizer.js:37:27:37:29 | url |
2820
| sanitizer.js:37:27:37:29 | url |
2921
| tst2.js:2:7:2:33 | href |
@@ -109,12 +101,6 @@ nodes
109101
edges
110102
| sanitizer.js:2:9:2:25 | url | sanitizer.js:4:27:4:29 | url |
111103
| sanitizer.js:2:9:2:25 | url | sanitizer.js:4:27:4:29 | url |
112-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:7:27:7:29 | url |
113-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:7:27:7:29 | url |
114-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:10:27:10:29 | url |
115-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:10:27:10:29 | url |
116-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:13:27:13:29 | url |
117-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:13:27:13:29 | url |
118104
| sanitizer.js:2:9:2:25 | url | sanitizer.js:16:27:16:29 | url |
119105
| sanitizer.js:2:9:2:25 | url | sanitizer.js:16:27:16:29 | url |
120106
| sanitizer.js:2:9:2:25 | url | sanitizer.js:19:27:19:29 | url |
@@ -127,8 +113,6 @@ edges
127113
| sanitizer.js:2:9:2:25 | url | sanitizer.js:28:27:28:29 | url |
128114
| sanitizer.js:2:9:2:25 | url | sanitizer.js:31:27:31:29 | url |
129115
| sanitizer.js:2:9:2:25 | url | sanitizer.js:31:27:31:29 | url |
130-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:34:27:34:29 | url |
131-
| sanitizer.js:2:9:2:25 | url | sanitizer.js:34:27:34:29 | url |
132116
| sanitizer.js:2:9:2:25 | url | sanitizer.js:37:27:37:29 | url |
133117
| sanitizer.js:2:9:2:25 | url | sanitizer.js:37:27:37:29 | url |
134118
| sanitizer.js:2:15:2:25 | window.name | sanitizer.js:2:9:2:25 | url |
@@ -209,16 +193,12 @@ edges
209193
| tst.js:6:34:6:55 | documen ... on.href | tst.js:6:20:6:56 | indirec ... n.href) |
210194
#select
211195
| sanitizer.js:4:27:4:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:4:27:4:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
212-
| sanitizer.js:7:27:7:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:7:27:7:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
213-
| sanitizer.js:10:27:10:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:10:27:10:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
214-
| sanitizer.js:13:27:13:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:13:27:13:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
215196
| sanitizer.js:16:27:16:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:16:27:16:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
216197
| sanitizer.js:19:27:19:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:19:27:19:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
217198
| sanitizer.js:22:27:22:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:22:27:22:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
218199
| sanitizer.js:25:27:25:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:25:27:25:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
219200
| sanitizer.js:28:27:28:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:28:27:28:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
220201
| sanitizer.js:31:27:31:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:31:27:31:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
221-
| sanitizer.js:34:27:34:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:34:27:34:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
222202
| sanitizer.js:37:27:37:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:37:27:37:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
223203
| tst2.js:4:21:4:55 | href.su ... '?')+1) | tst2.js:2:14:2:28 | window.location | tst2.js:4:21:4:55 | href.su ... '?')+1) | Untrusted URL redirection due to $@. | tst2.js:2:14:2:28 | window.location | user-provided value |
224204
| tst6.js:4:21:4:28 | redirect | tst6.js:2:18:2:45 | $locati ... irect') | tst6.js:4:21:4:28 | redirect | Untrusted URL redirection due to $@. | tst6.js:2:18:2:45 | $locati ... irect') | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/sanitizer.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ function f() {
44
window.location = url; // NOT OK - can be example.com.evil.com
55
}
66
if (url.startsWith('https://example.com/')) {
7-
window.location = url; // OK - but flagged anyway
7+
window.location = url; // OK
88
}
99
if (url.startsWith('https://example.com//')) {
10-
window.location = url; // OK - but flagged anyway
10+
window.location = url; // OK
1111
}
1212
if (url.startsWith('https://example.com/foo')) {
13-
window.location = url; // OK - but flagged anyway
13+
window.location = url; // OK
1414
}
1515
if (url.startsWith('https://')) {
1616
window.location = url; // NOT OK - does not restrict hostname
@@ -31,9 +31,13 @@ function f() {
3131
window.location = url; // NOT OK - can be //example.com.evil.com
3232
}
3333
if (url.startsWith('//example.com/')) {
34-
window.location = url; // OK - but flagged anyway
34+
window.location = url; // OK
3535
}
3636
if (url.endsWith('https://example.com/')) {
3737
window.location = url; // NOT OK - could be evil.com?x=https://example.com/
3838
}
39+
let basedir = whatever() ? 'foo' : 'bar';
40+
if (url.startsWith('https://example.com/' + basedir)) {
41+
window.location = url; // OK - the whole prefix is not known, but enough to restrict hostname
42+
}
3943
}

0 commit comments

Comments
 (0)