Skip to content

Commit 2f8c9a5

Browse files
authored
Merge pull request github#12171 from erik-krogh/reg-dot
JS: dont recognize regexps that match dot as sanitizers
2 parents e3e2df3 + 4140598 commit 2f8c9a5

File tree

4 files changed

+50
-2
lines changed

4 files changed

+50
-2
lines changed

javascript/ql/lib/semmle/javascript/MembershipCandidates.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ module MembershipCandidate {
147147
child instanceof RegExpConstant or
148148
child instanceof RegExpAlt or
149149
child instanceof RegExpGroup
150-
)
150+
) and
151+
// exclude "length matches" that match every string
152+
not this.getAChild*() instanceof RegExpDot
151153
}
152154

153155
/**

javascript/ql/test/experimental/Security/CWE-918/SSRF.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ nodes
3131
| check-path.js:45:13:45:44 | `${base ... inted}` |
3232
| check-path.js:45:26:45:42 | req.query.tainted |
3333
| check-path.js:45:26:45:42 | req.query.tainted |
34+
| check-regex.js:16:15:16:45 | "test.c ... tainted |
35+
| check-regex.js:16:15:16:45 | "test.c ... tainted |
36+
| check-regex.js:16:29:16:45 | req.query.tainted |
37+
| check-regex.js:16:29:16:45 | req.query.tainted |
3438
| check-regex.js:24:15:24:42 | baseURL ... tainted |
3539
| check-regex.js:24:15:24:42 | baseURL ... tainted |
3640
| check-regex.js:24:25:24:42 | req.params.tainted |
@@ -103,6 +107,10 @@ edges
103107
| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
104108
| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
105109
| check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` |
110+
| check-regex.js:16:29:16:45 | req.query.tainted | check-regex.js:16:15:16:45 | "test.c ... tainted |
111+
| check-regex.js:16:29:16:45 | req.query.tainted | check-regex.js:16:15:16:45 | "test.c ... tainted |
112+
| check-regex.js:16:29:16:45 | req.query.tainted | check-regex.js:16:15:16:45 | "test.c ... tainted |
113+
| check-regex.js:16:29:16:45 | req.query.tainted | check-regex.js:16:15:16:45 | "test.c ... tainted |
106114
| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
107115
| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
108116
| check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted |
@@ -153,6 +161,7 @@ edges
153161
| check-path.js:33:15:33:45 | 'test.c ... tainted | check-path.js:33:29:33:45 | req.query.tainted | check-path.js:33:15:33:45 | 'test.c ... tainted | The URL of this request depends on a user-provided value. |
154162
| check-path.js:37:15:37:45 | 'test.c ... tainted | check-path.js:37:29:37:45 | req.query.tainted | check-path.js:37:15:37:45 | 'test.c ... tainted | The URL of this request depends on a user-provided value. |
155163
| check-path.js:45:13:45:44 | `${base ... inted}` | check-path.js:45:26:45:42 | req.query.tainted | check-path.js:45:13:45:44 | `${base ... inted}` | The URL of this request depends on a user-provided value. |
164+
| check-regex.js:16:15:16:45 | "test.c ... tainted | check-regex.js:16:29:16:45 | req.query.tainted | check-regex.js:16:15:16:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
156165
| check-regex.js:24:15:24:42 | baseURL ... tainted | check-regex.js:24:25:24:42 | req.params.tainted | check-regex.js:24:15:24:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |
157166
| check-regex.js:31:15:31:45 | "test.c ... tainted | check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
158167
| check-regex.js:34:15:34:42 | baseURL ... tainted | check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,14 @@ nodes
293293
| lib/lib.js:555:25:555:37 | ["-rf", name] |
294294
| lib/lib.js:555:33:555:36 | name |
295295
| lib/lib.js:555:33:555:36 | name |
296+
| lib/lib.js:558:41:558:44 | name |
297+
| lib/lib.js:558:41:558:44 | name |
298+
| lib/lib.js:560:26:560:29 | name |
299+
| lib/lib.js:560:26:560:29 | name |
300+
| lib/lib.js:562:26:562:29 | name |
301+
| lib/lib.js:562:26:562:29 | name |
302+
| lib/lib.js:566:26:566:29 | name |
303+
| lib/lib.js:566:26:566:29 | name |
296304
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
297305
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
298306
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -683,6 +691,18 @@ edges
683691
| lib/lib.js:551:33:551:36 | args | lib/lib.js:552:23:552:26 | args |
684692
| lib/lib.js:555:25:555:37 | ["-rf", name] | lib/lib.js:551:33:551:36 | args |
685693
| lib/lib.js:555:33:555:36 | name | lib/lib.js:555:25:555:37 | ["-rf", name] |
694+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name |
695+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name |
696+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name |
697+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name |
698+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name |
699+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name |
700+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name |
701+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name |
702+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
703+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
704+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
705+
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
686706
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
687707
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
688708
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -803,6 +823,9 @@ edges
803823
| lib/lib.js:545:11:545:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:545:3:545:27 | cp.exec ... + name) | shell command |
804824
| lib/lib.js:552:23:552:26 | args | lib/lib.js:550:39:550:42 | name | lib/lib.js:552:23:552:26 | args | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:550:39:550:42 | name | library input | lib/lib.js:552:9:552:38 | cp.spaw ... wnOpts) | shell command |
805825
| lib/lib.js:555:33:555:36 | name | lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:550:39:550:42 | name | library input | lib/lib.js:552:9:552:38 | cp.spaw ... wnOpts) | shell command |
826+
| lib/lib.js:560:14:560:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:560:9:560:30 | exec("r ... + name) | shell command |
827+
| lib/lib.js:562:14:562:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:562:9:562:30 | exec("r ... + name) | shell command |
828+
| lib/lib.js:566:14:566:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:566:9:566:30 | exec("r ... + name) | shell command |
806829
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
807830
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
808831
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,4 +553,18 @@ module.exports.shellThing = function (name) {
553553
}
554554

555555
indirectShell("rm", ["-rf", name], {shell: true});
556-
}
556+
}
557+
558+
module.exports.badSanitizer = function (name) {
559+
if (!name.match(/^(.|\.){1,64}$/)) { // <- bad sanitizer
560+
exec("rm -rf " + name); // NOT OK
561+
} else {
562+
exec("rm -rf " + name); // NOT OK
563+
}
564+
565+
if (!name.match(/^\w{1,64}$/)) { // <- good sanitizer
566+
exec("rm -rf " + name); // NOT OK
567+
} else {
568+
exec("rm -rf " + name); // OK
569+
}
570+
}

0 commit comments

Comments
 (0)