diff --git a/javascript/change-notes/2021-01-08-js-incomplete-multi-character-sanitization.md b/javascript/change-notes/2021-01-08-js-incomplete-multi-character-sanitization.md new file mode 100644 index 00000000000..653ba2a8994 --- /dev/null +++ b/javascript/change-notes/2021-01-08-js-incomplete-multi-character-sanitization.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query "Incomplete multi-character sanitization" (`js/incomplete-multi-character-sanitization`) has been improved to produce additional true positives and fewer false positives. diff --git a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql index dbff9d98bc5..cef10d3cbe7 100644 --- a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql +++ b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql @@ -12,69 +12,153 @@ */ import javascript -import semmle.javascript.security.IncompleteBlacklistSanitizer -predicate isDangerous(RegExpTerm t) { - // path traversals - t.getAMatchedString() = ["..", "/..", "../"] - or - exists(RegExpTerm start | - start = t.(RegExpSequence).getAChild() and - start.getConstantValue() = "." and - start.getSuccessor().getConstantValue() = "." and - not [start.getPredecessor(), start.getSuccessor().getSuccessor()].getConstantValue() = "." - ) - or - // HTML comments - t.getAMatchedString() = "/gm, ""); // NOT OK - x = x.replace(/\sng-[a-z-]+/, ""); // OK (single ng-attribute, should be flagged by some other query!) + x = x.replace(/\sng-[a-z-]+/, ""); // NOT OK x = x.replace(/\sng-[a-z-]+/g, ""); // NOT OK (ng-attributes) x = x.replace(/()/g, "\n"); // OK: not a sanitizer - x = x.replace(//g, ""); // OK, but still flagged [INCONSISTENCY] - x = x.replace(/