JS: initial version of IncompleteMultiCharacterSanitization.ql

This commit is contained in:
Esben Sparre Andreasen
2020-06-08 10:20:26 +02:00
parent 8b3dd6dec4
commit 2d2468463b
5 changed files with 255 additions and 0 deletions

View File

@@ -0,0 +1,24 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
</overview>
<recommendation>
</recommendation>
<example>
</example>
<references>
<li>OWASP Top 10: <a href="https://www.owasp.org/index.php/Top_10-2017_A1-Injection">A1 Injection</a>.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,81 @@
/**
* @name Incomplete multi-character sanitization
* @description A sanitizer that removes a sequence of characters may reintroduce the dangerous sequence.
* @kind problem
* @problem.severity warning
* @precision high
* @id js/incomplete-multi-character-sanitization
* @tags correctness
* security
* external/cwe/cwe-116
* external/cwe/cwe-20
*/
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() = "<!--"
or
// HTML scripts
t.getAMatchedString().regexpMatch("(?i)<script.*")
or
exists(RegExpSequence seq | seq = t |
t.getChild(0).getConstantValue() = "<" and
// the `cript|scrip` case has been observed in the wild, not sure what the goal of that pattern is...
t
.getChild(0)
.getSuccessor+()
.getAMatchedString()
.regexpMatch("(?i)iframe|script|cript|scrip|style")
)
or
// HTML attributes
exists(string dangerousPrefix | dangerousPrefix = ["ng-", "on"] |
t.getAMatchedString().regexpMatch("(i?)" + dangerousPrefix + "[a-z]+")
or
exists(RegExpTerm start, RegExpTerm event | start = t.getAChild() |
start.getConstantValue().regexpMatch("(?i)[^a-z]*" + dangerousPrefix) and
event = start.getSuccessor() and
exists(RegExpTerm quantified | quantified = event.(RegExpQuantifier).getChild(0) |
quantified
.(RegExpCharacterClass)
.getAChild()
.(RegExpCharacterRange)
.isRange(["a", "A"], ["z", "Z"]) or
[quantified, quantified.(RegExpRange).getAChild()].(RegExpCharacterClassEscape).getValue() =
"w"
)
)
)
}
from StringReplaceCall replace, RegExpTerm regexp, RegExpTerm dangerous
where
[replace.getRawReplacement(), replace.getCallback(1).getAReturn()].mayHaveStringValue("") and
replace.isGlobal() and
regexp = replace.getRegExp().getRoot() and
dangerous.getRootTerm() = regexp and
isDangerous(dangerous) and
// avoid anchored terms
not exists(RegExpAnchor a | a.getRootTerm() = regexp) and
// avoid flagging wrappers
not (
dangerous instanceof RegExpAlt or
dangerous instanceof RegExpGroup
) and
// don't flag replace operations in a loop
not replace.getReceiver() = replace.getASuccessor+()
select replace, "The replaced string may still contain a substring that starts matching at $@.",
dangerous, dangerous.toString()

View File

@@ -0,0 +1,33 @@
| tst-multi-character-sanitization.js:3:13:3:57 | content ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:3:30:3:49 | <.*cript.*\\/scrip.*> | <.*cript.*\\/scrip.*> |
| tst-multi-character-sanitization.js:4:13:4:47 | content ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:4:30:4:40 | on\\w+=".*" | on\\w+=".*" |
| tst-multi-character-sanitization.js:5:13:5:49 | content ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:5:30:5:42 | on\\w+=\\'.*\\' | on\\w+=\\'.*\\' |
| tst-multi-character-sanitization.js:9:13:9:47 | content ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:9:30:9:39 | <.*cript.* | <.*cript.* |
| tst-multi-character-sanitization.js:10:13:10:49 | content ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:10:30:10:42 | .on\\w+=.*".*" | .on\\w+=.*".*" |
| tst-multi-character-sanitization.js:11:13:11:51 | content ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:11:30:11:44 | .on\\w+=.*\\'.*\\' | .on\\w+=.*\\'.*\\' |
| tst-multi-character-sanitization.js:19:3:19:35 | respons ... pt, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:18:18:18:24 | <script | <script |
| tst-multi-character-sanitization.js:25:10:25:40 | text.re ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:25:24:25:27 | <!-- | <!-- |
| tst-multi-character-sanitization.js:38:8:38:30 | id.repl ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:38:20:38:23 | \\.\\. | \\.\\. |
| tst-multi-character-sanitization.js:49:13:49:43 | req.url ... EL, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:48:21:48:31 | (\\/)?\\.\\.\\/ | (\\/)?\\.\\.\\/ |
| tst-multi-character-sanitization.js:64:7:64:73 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:64:18:64:24 | <script | <script |
| tst-multi-character-sanitization.js:66:7:66:56 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:66:18:66:49 | (\\/\|\\s)on\\w+=(\\'\|")?[^"]*(\\'\|")? | (\\/\|\\s)on\\w+=(\\'\|")?[^"]*(\\'\|")? |
| tst-multi-character-sanitization.js:75:7:75:37 | x.repla ... gm, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:75:18:75:21 | <!-- | <!-- |
| tst-multi-character-sanitization.js:77:7:77:36 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:77:18:77:29 | \\sng-[a-z-]+ | \\sng-[a-z-]+ |
| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:81:18:81:24 | <script | <script |
| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:81:36:81:39 | only | only |
| tst-multi-character-sanitization.js:82:7:82:50 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:82:18:82:30 | <script async | <script async |
| tst-multi-character-sanitization.js:83:7:83:63 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:83:18:83:21 | <!-- | <!-- |
| tst-multi-character-sanitization.js:85:7:85:48 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:85:18:85:41 | \\x2E\\x2E\\x2F\\x2E\\x2E\\x2F | \\x2E\\x2E\\x2F\\x2E\\x2E\\x2F |
| tst-multi-character-sanitization.js:87:7:87:47 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:87:18:87:24 | <script | <script |
| tst-multi-character-sanitization.js:92:7:96:4 | x.repla ... ";\\n }) | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:92:18:92:24 | <script | <script |
| tst-multi-character-sanitization.js:100:7:100:28 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:100:18:100:21 | \\.\\. | \\.\\. |
| tst-multi-character-sanitization.js:101:7:101:30 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:101:18:101:23 | \\.\\.\\/ | \\.\\.\\/ |
| tst-multi-character-sanitization.js:102:7:102:30 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:102:18:102:23 | \\/\\.\\. | \\/\\.\\. |
| tst-multi-character-sanitization.js:104:7:104:58 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:104:18:104:24 | <script | <script |
| tst-multi-character-sanitization.js:106:7:106:64 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:106:18:106:56 | <(script\|del)(?=[\\s>])[\\w\\W]*?<\\/\\1\\s*> | <(script\|del)(?=[\\s>])[\\w\\W]*?<\\/\\1\\s*> |
| tst-multi-character-sanitization.js:107:7:107:62 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:107:18:107:55 | \\<script[\\s\\S]*?\\>[\\s\\S]*?\\<\\/script\\> | \\<script[\\s\\S]*?\\>[\\s\\S]*?\\<\\/script\\> |
| tst-multi-character-sanitization.js:108:7:108:75 | x.repla ... gm, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:108:18:108:67 | <(script\|style\|title)[^<]+<\\/(script\|style\|title)> | <(script\|style\|title)[^<]+<\\/(script\|style\|title)> |
| tst-multi-character-sanitization.js:109:7:109:58 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:109:18:109:24 | <script | <script |
| tst-multi-character-sanitization.js:110:7:110:50 | x.repla ... gi, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:110:18:110:24 | <script | <script |
| tst-multi-character-sanitization.js:111:7:111:32 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:111:20:111:23 | <!-- | <!-- |
| tst-multi-character-sanitization.js:112:7:112:50 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:112:18:112:43 | require\\('\\.\\.\\/common'\\); | require\\('\\.\\.\\/common'\\); |
| tst-multi-character-sanitization.js:113:7:113:41 | x.repla ... /g, "") | The replaced string may still contain a substring that starts matching at $@. | tst-multi-character-sanitization.js:113:18:113:34 | \\.\\.\\/\\.\\.\\/lib\\/ | \\.\\.\\/\\.\\.\\/lib\\/ |

View File

@@ -0,0 +1 @@
Security/CWE-116/IncompleteMultiCharacterSanitization.ql

View File

@@ -0,0 +1,116 @@
// CVE-2019-10756
(function(content) {
content = content.replace(/<.*cript.*\/scrip.*>/gi, ""); // NOT OK
content = content.replace(/ on\w+=".*"/g, ""); // NOT OK
content = content.replace(/ on\w+=\'.*\'/g, ""); // NOT OK
return content;
});
(function(content) {
content = content.replace(/<.*cript.*/gi, ""); // NOT OK
content = content.replace(/.on\w+=.*".*"/g, ""); // NOT OK
content = content.replace(/.on\w+=.*\'.*\'/g, ""); // NOT OK
return content;
});
// CVE-2020-7656
(function(responseText) {
var rscript = /<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi;
responseText.replace(rscript, ""); // NOT OK
return responseText;
});
// CVE-2019-1010091
(function(text) {
text = text.replace(/<!--|--!?>/g, ""); // NOT OK
return text;
});
(function(text) {
while (/<!--|--!?>/g.test(text)) {
text = text.replace(/<!--|--!?>/g, ""); // OK
}
return text;
});
// CVE-2019-10767
(function(id) {
id = id.replace(/\.\./g, ""); // NOT OK
return id;
});
(function(id) {
id = id.replace(/[\]\[*,;'"`<>\\?\/]/g, ""); // OK (or is it?)
return id;
});
// CVE-2019-8903
(function(req) {
var REG_TRAVEL = /(\/)?\.\.\//g;
req.url = req.url.replace(REG_TRAVEL, ""); // NOT OK
});
(function(req) {
var beg;
for (var i = 0; i < req.url.length; i++) {
if (req.url[i] === "." && req.url[i + 1] === "/") beg = i + 1;
else if (req.url[i] === "?") break;
}
if (beg) req.url = req.url.substring(beg);
});
// New cases
(function(x) {
x = x.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/g, ""); // NOT OK
x = x.replace(/(\/|\s)on\w+=(\'|")?[^"]*(\'|")?/g, ""); // NOT OK
x = x.replace(/<\/script>/g, ""); // OK
x = x.replace(/<(.)?br(.)?>/g, ""); // OK
x = x.replace(/<\/?b>/g, ""); // OK
x = x.replace(/<(ul|ol)><\/(ul|ol)>/gi, ""); // OK
x = x.replace(/<li><\/li>/gi, ""); // OK
x = x.replace(/<!--(.*?)-->/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-]+/g, ""); // NOT OK (ng-attributes)
x = x.replace(/(<!--\[CDATA\[|\]\]-->)/g, "\n"); // OK: not a sanitizer
x = x.replace(/<script.+desktop\-only.+<\/script>/g, ""); // OK, but still flagged [INCONSISTENCY]
x = x.replace(/<script async.+?<\/script>/g, ""); // OK, but still flagged [INCONSISTENCY]
x = x.replace(/<!--[\s\S]*?-->|<\?(?:php)?[\s\S]*?\?>/gi, ""); // NOT OK
x = x.replace(/\x2E\x2E\x2F\x2E\x2E\x2F/g, ""); // NOT OK (matches "../../")
x = x.replace(/<script.*>.*<\/script>/gi, ""); // NOT OK
x = x.replace(/^(\.\.\/?)+/g, ""); // OK
// NOT OK
x = x.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/g, function(
$0
) {
return unknown ? $0 : "";
});
x = x.replace(/<\/?([a-z][a-z0-9]*)\b[^>]*>/gi, ""); // NOT OK [INCONSISTENCY]
x = x.replace(/\.\./g, ""); // NOT OK
x = x.replace(/\.\.\//g, ""); // NOT OK
x = x.replace(/\/\.\./g, ""); // NOT OK
x = x.replace(/<script(.*?)>([\s\S]*?)<\/script>/gi, ""); // NOT OK
x = x.replace(/<(script|del)(?=[\s>])[\w\W]*?<\/\1\s*>/gi, ""); // NOT OK
x = x.replace(/\<script[\s\S]*?\>[\s\S]*?\<\/script\>/g, ""); // NOT OK
x = x.replace(/<(script|style|title)[^<]+<\/(script|style|title)>/gm, ""); // NOT OK
x = x.replace(/<script[^>]*>([\s\S]*?)<\/script>/gi, ""); // NOT OK
x = x.replace(/<script[\s\S]*?<\/script>/gi, ""); // NOT OK
x = x.replace(/ ?<!-- ?/g, ""); // NOT OK
x = x.replace(/require\('\.\.\/common'\);/g, ""); // OK [INCONSISTENCY] permit alphanum-suffix after the dots?
x = x.replace(/\.\.\/\.\.\/lib\//g, ""); // OK [INCONSISTENCY] permit alphanum-suffix after the dots?
return x;
});