Skip to content

Commit 364ba1b

Browse files
author
Esben Sparre Andreasen
committed
JS: use RegExpLiteral as a SourceNode
1 parent 7923c9d commit 364ba1b

File tree

8 files changed

+16
-5
lines changed

8 files changed

+16
-5
lines changed

javascript/ql/src/RegExp/IdentityReplacement.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import javascript
1818
*/
1919
predicate matchesString(Expr e, string s) {
2020
exists(RegExpLiteral rl |
21-
rl = e and
21+
rl.flow().(DataFlow::SourceNode).flowsToExpr(e) and
2222
not rl.isIgnoreCase() and
2323
regExpMatchesString(rl.getRoot(), s)
2424
)

javascript/ql/src/Security/CWE-116/DoubleEscaping.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class Replacement extends DataFlow::Node {
7070
Replacement() {
7171
exists(DataFlow::MethodCallNode mcn | this = mcn |
7272
mcn.getMethodName() = "replace" and
73-
mcn.getArgument(0).asExpr() = pattern and
73+
pattern.flow().(DataFlow::SourceNode).flowsTo(mcn.getArgument(0))and
7474
mcn.getNumArgument() = 2 and
7575
pattern.isGlobal()
7676
)

javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ predicate isSimple(RegExpTerm t) {
5959
*/
6060
predicate isBackslashEscape(MethodCallExpr mce, RegExpLiteral re) {
6161
mce.getMethodName() = "replace" and
62-
re = mce.getArgument(0) and
62+
re.flow().(DataFlow::SourceNode).flowsToExpr(mce.getArgument(0)) and
6363
re.isGlobal() and
6464
exists(string new | new = mce.getArgument(1).getStringValue() |
6565
// `new` is `\$&`, `\$1` or similar
@@ -104,7 +104,7 @@ predicate allBackslashesEscaped(DataFlow::Node nd) {
104104
from MethodCallExpr repl, Expr old, string msg
105105
where
106106
repl.getMethodName() = "replace" and
107-
old = repl.getArgument(0) and
107+
(old = repl.getArgument(0) or old.flow().(DataFlow::SourceNode).flowsToExpr(repl.getArgument(0))) and
108108
(
109109
not old.(RegExpLiteral).isGlobal() and
110110
msg = "This replaces only the first occurrence of " + old + "." and

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ module ClientSideUrlRedirect {
100100
or
101101
exists(MethodCallExpr mce |
102102
queryAccess.asExpr() = mce and
103-
mce.calls(any(RegExpLiteral re), "exec") and
103+
mce = any(RegExpLiteral re).flow().(DataFlow::SourceNode).getAMethodCall("exec").asExpr() and
104104
nd.asExpr() = mce.getArgument(0)
105105
)
106106
}

javascript/ql/test/query-tests/RegExp/IdentityReplacement/IdentityReplacement.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| IdentityReplacement.js:1:27:1:30 | /"/g | This replaces '"' with itself. |
2+
| IdentityReplacement.js:4:14:4:21 | indirect | This replaces '"' with itself. |
23
| tst.js:1:13:1:16 | "\\\\" | This replaces '\\' with itself. |
34
| tst.js:2:13:2:18 | /(\\\\)/ | This replaces '\\' with itself. |
45
| tst.js:3:13:3:17 | /["]/ | This replaces '"' with itself. |

javascript/ql/test/query-tests/Security/CWE-116/DoubleEscaping/DoubleEscaping.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44
| tst.js:47:7:47:30 | s.repla ... g, "&") | This replacement may produce '&' characters that are double-unescaped $@. | tst.js:48:7:48:32 | s.repla ... , "\\"") | here |
55
| tst.js:53:10:53:33 | s.repla ... , '\\\\') | This replacement may produce '\\' characters that are double-unescaped $@. | tst.js:53:10:54:33 | s.repla ... , '\\'') | here |
66
| tst.js:60:7:60:28 | s.repla ... '%25') | This replacement may double-escape '%' characters from $@. | tst.js:59:7:59:28 | s.repla ... '%26') | here |
7+
| tst.js:68:10:70:38 | s.repla ... &") | This replacement may double-escape '&' characters from $@. | tst.js:68:10:69:39 | s.repla ... apos;") | here |

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteSanitization.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
| tst.js:61:10:61:18 | s.replace | This replaces only the first occurrence of "'" + "". |
1616
| tst.js:65:10:65:18 | s.replace | This replaces only the first occurrence of "'". |
1717
| tst.js:69:10:69:18 | s.replace | This replaces only the first occurrence of "'" + "". |
18+
| tst.js:169:9:169:17 | s.replace | This replaces only the first occurrence of /'/. |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ nodes
3838
| tst.js:2:19:2:72 | /.*redi ... ref)[1] |
3939
| tst.js:2:47:2:63 | document.location |
4040
| tst.js:2:47:2:68 | documen ... on.href |
41+
| tst.js:6:20:6:56 | indirec ... n.href) |
42+
| tst.js:6:20:6:59 | indirec ... ref)[1] |
43+
| tst.js:6:34:6:50 | document.location |
44+
| tst.js:6:34:6:55 | documen ... on.href |
4145
edges
4246
| tst2.js:2:7:2:33 | href | tst2.js:4:21:4:24 | href |
4347
| tst2.js:2:7:2:33 | href | tst2.js:4:21:4:24 | href |
@@ -69,6 +73,9 @@ edges
6973
| tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
7074
| tst.js:2:47:2:63 | document.location | tst.js:2:47:2:68 | documen ... on.href |
7175
| tst.js:2:47:2:68 | documen ... on.href | tst.js:2:19:2:69 | /.*redi ... n.href) |
76+
| tst.js:6:20:6:56 | indirec ... n.href) | tst.js:6:20:6:59 | indirec ... ref)[1] |
77+
| tst.js:6:34:6:50 | document.location | tst.js:6:34:6:55 | documen ... on.href |
78+
| tst.js:6:34:6:55 | documen ... on.href | tst.js:6:20:6:56 | indirec ... n.href) |
7279
#select
7380
| 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 |
7481
| 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 |
@@ -84,3 +91,4 @@ edges
8491
| tst10.js:11:17:11:50 | '//foo' ... .search | tst10.js:11:27:11:43 | document.location | tst10.js:11:17:11:50 | '//foo' ... .search | Untrusted URL redirection due to $@. | tst10.js:11:27:11:43 | document.location | user-provided value |
8592
| tst10.js:14:17:14:56 | 'https: ... .search | tst10.js:14:33:14:49 | document.location | tst10.js:14:17:14:56 | 'https: ... .search | Untrusted URL redirection due to $@. | tst10.js:14:33:14:49 | document.location | user-provided value |
8693
| tst.js:2:19:2:72 | /.*redi ... ref)[1] | tst.js:2:47:2:63 | document.location | tst.js:2:19:2:72 | /.*redi ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:2:47:2:63 | document.location | user-provided value |
94+
| tst.js:6:20:6:59 | indirec ... ref)[1] | tst.js:6:34:6:50 | document.location | tst.js:6:20:6:59 | indirec ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:6:34:6:50 | document.location | user-provided value |

0 commit comments

Comments
 (0)