Skip to content

Commit 42fc28b

Browse files
author
Esben Sparre Andreasen
committed
JS: add ad hoc whitelist checks as sanitizers
1 parent 89f2dbf commit 42fc28b

File tree

5 files changed

+41
-0
lines changed

5 files changed

+41
-0
lines changed

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,28 @@ module TaintTracking {
625625

626626
}
627627

628+
/**
629+
* A check of the form `if(<isWhitelisted>(x))`, which sanitizes `x` in its "then" branch.
630+
*
631+
* `<isWhitelisted>` is a call with callee name 'safe', 'whitelist', 'allow', or similar.
632+
*/
633+
private class AdHocWhitelistCheckSanitizer extends AdditionalSanitizerGuardNode, DataFlow::CallNode {
634+
AdHocWhitelistCheckSanitizer() {
635+
getCalleeName().regexpMatch("(?i).*(safe|whitelist|allow|auth).*") and
636+
getNumArgument() = 1
637+
}
638+
639+
override predicate sanitizes(boolean outcome, Expr e) {
640+
outcome = true and
641+
e = getArgument(0).asExpr()
642+
}
643+
644+
override predicate appliesTo(Configuration cfg) {
645+
any()
646+
}
647+
648+
}
649+
628650
/** A check of the form `if(x in o)`, which sanitizes `x` in its "then" branch. */
629651
class InSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
630652

javascript/ql/test/library-tests/TaintBarriers/SanitizingGuard.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,5 @@
3636
| tst.js:214:9:214:24 | o.indexOf(v) < 0 | ExampleConfiguration | false | tst.js:214:19:214:19 | v |
3737
| tst.js:220:9:220:25 | o.indexOf(v) > -1 | ExampleConfiguration | true | tst.js:220:19:220:19 | v |
3838
| tst.js:226:9:226:26 | -1 >= o.indexOf(v) | ExampleConfiguration | false | tst.js:226:25:226:25 | v |
39+
| tst.js:236:9:236:24 | isWhitelisted(v) | ExampleConfiguration | true | tst.js:236:23:236:23 | v |
40+
| tst.js:240:9:240:28 | config.allowValue(v) | ExampleConfiguration | true | tst.js:240:27:240:27 | v |

javascript/ql/test/library-tests/TaintBarriers/TaintedSink.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,5 @@
3434
| tst.js:215:14:215:14 | v | tst.js:199:13:199:20 | SOURCE() |
3535
| tst.js:223:14:223:14 | v | tst.js:199:13:199:20 | SOURCE() |
3636
| tst.js:227:14:227:14 | v | tst.js:199:13:199:20 | SOURCE() |
37+
| tst.js:239:14:239:14 | v | tst.js:235:13:235:20 | SOURCE() |
38+
| tst.js:243:14:243:14 | v | tst.js:235:13:235:20 | SOURCE() |

javascript/ql/test/library-tests/TaintBarriers/isBarrier.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,5 @@
2929
| tst.js:217:14:217:14 | v | ExampleConfiguration |
3030
| tst.js:221:14:221:14 | v | ExampleConfiguration |
3131
| tst.js:229:14:229:14 | v | ExampleConfiguration |
32+
| tst.js:237:14:237:14 | v | ExampleConfiguration |
33+
| tst.js:241:14:241:14 | v | ExampleConfiguration |

javascript/ql/test/library-tests/TaintBarriers/tst.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,3 +230,16 @@ function RelationalIndexOfCheckSanitizer () {
230230
}
231231

232232
}
233+
234+
function adhocWhitelisting() {
235+
var v = SOURCE();
236+
if (isWhitelisted(v))
237+
SINK(v);
238+
else
239+
SINK(v);
240+
if (config.allowValue(v))
241+
SINK(v);
242+
else
243+
SINK(v);
244+
245+
}

0 commit comments

Comments
 (0)