hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

sanitize variables used in an HTML escaping switch-case

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-05-27 20:26:01 +02:00
parent 1a2db10a90
commit 5bb308dc8f
4 changed files with 208 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
View File

@@ -93,6 +93,53 @@ module Shared {
}
}
/**
* Holds if `str` is used in a switch-case that has cases matching HTML escaping.
*/
private predicate isUsedInHTMLEscapingSwitch(Expr str) {
exists(SwitchStmt switch |
// "\"".charCodeAt(0) == 34, "&".charCodeAt(0) == 38, "<".charCodeAt(0) == 60
forall(int c | c = [34, 38, 60] | c = switch.getACase().getExpr().getIntValue()) and
exists(DataFlow::MethodCallNode mcn | mcn.getMethodName() = "charCodeAt" |
mcn.flowsToExpr(switch.getExpr()) and
str = mcn.getReceiver().asExpr()
)
or
forall(string c | c = ["\"", "&", "<"] | c = switch.getACase().getExpr().getStringValue()) and
(
exists(DataFlow::MethodCallNode mcn | mcn.getMethodName() = "charAt" |
mcn.flowsToExpr(switch.getExpr()) and
str = mcn.getReceiver().asExpr()
)
or
exists(DataFlow::PropRead read | exists(read.getPropertyNameExpr()) |
read.flowsToExpr(switch.getExpr()) and
str = read.getBase().asExpr()
)
)
)
}
private import semmle.javascript.dataflow.internal.AccessPaths as Paths
/**
* Gets an access-path that is used in a sanitizing switch statement.
* The `pragma[noinline]` is to avoid materializing a cartesian product of all access-paths.
*/
pragma[noinline]
private Paths::AccessPath getAPathEscapedInSwitch() {
exists(Expr str |
isUsedInHTMLEscapingSwitch(str) and
result.getAnInstance() = str
)
}
/**
* An expression that is sanitized by a switch-case.
*/
class IsEscapedInSwitchSanitizer extends Sanitizer {
IsEscapedInSwitchSanitizer() { this.asExpr() = getAPathEscapedInSwitch().getAnInstance() }
}
}
/** Provides classes and predicates for the DOM-based XSS query. */
@@ -348,6 +395,8 @@ module DomBasedXss {
private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
private class IsEscapedInSwitchSanitizer extends Sanitizer, Shared::IsEscapedInSwitchSanitizer { }
private class QuoteGuard extends SanitizerGuard, Shared::QuoteGuard { }
/**
@@ -484,6 +533,8 @@ module ReflectedXss {
private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
private class IsEscapedInSwitchSanitizer extends Sanitizer, Shared::IsEscapedInSwitchSanitizer { }
private class QuoteGuard extends SanitizerGuard, Shared::QuoteGuard { }
private class ContainsHTMLGuard extends SanitizerGuard, Shared::ContainsHTMLGuard { }
@@ -519,6 +570,8 @@ module StoredXss {
private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
private class IsEscapedInSwitchSanitizer extends Sanitizer, Shared::IsEscapedInSwitchSanitizer { }
private class QuoteGuard extends SanitizerGuard, Shared::QuoteGuard { }
private class ContainsHTMLGuard extends SanitizerGuard, Shared::ContainsHTMLGuard { }

12
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected
View File

@@ -19,6 +19,12 @@ nodes
| ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:70:22:70:34 | req.params.id |
| ReflectedXssContentTypes.js:70:22:70:34 | req.params.id |
| ReflectedXssGood3.js:135:9:135:27 | url |
| ReflectedXssGood3.js:135:15:135:27 | req.params.id |
| ReflectedXssGood3.js:135:15:135:27 | req.params.id |
| ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) |
| ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) |
| ReflectedXssGood3.js:139:24:139:26 | url |
| etherpad.js:9:5:9:53 | response |
| etherpad.js:9:16:9:30 | req.query.jsonp |
| etherpad.js:9:16:9:30 | req.query.jsonp |
@@ -105,6 +111,11 @@ edges
| ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id |
| ReflectedXssGood3.js:135:9:135:27 | url | ReflectedXssGood3.js:139:24:139:26 | url |
| ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:135:9:135:27 | url |
| ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:135:9:135:27 | url |
| ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) |
| ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) |
| etherpad.js:9:5:9:53 | response | etherpad.js:11:12:11:19 | response |
| etherpad.js:9:5:9:53 | response | etherpad.js:11:12:11:19 | response |
| etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:9:16:9:53 | req.que ... e + ")" |
@@ -166,6 +177,7 @@ edges
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | user-provided value |
| ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | Cross-site scripting vulnerability due to $@. | ReflectedXssGood3.js:135:15:135:27 | req.params.id | user-provided value |
| etherpad.js:11:12:11:19 | response | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value |
| exception-xss.js:190:12:190:24 | req.params.id | exception-xss.js:190:12:190:24 | req.params.id | exception-xss.js:190:12:190:24 | req.params.id | Cross-site scripting vulnerability due to $@. | exception-xss.js:190:12:190:24 | req.params.id | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |

142
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXssGood3.js Normal file
View File

@@ -0,0 +1,142 @@
var express = require('express');
var app = express();
function escapeHtml1(string) {
var str = "" + string;
let escape;
let html = '';
let lastIndex = 0;
for (let index = 0; index < str.length; index++) {
switch (str.charCodeAt(index)) {
case 34: // "
escape = '&quot;';
break;
case 38: // &
escape = '&amp;';
break;
case 39: // '
escape = '&#39;';
break;
case 60: // <
escape = '&lt;';
break;
case 62: // >
escape = '&gt;';
break;
default:
continue;
}
if (lastIndex !== index) {
html += str.substring(lastIndex, index);
}
lastIndex = index + 1;
html += escape;
}
return lastIndex !== index
? html + str.substring(lastIndex, index)
: html;
}
function escapeHtml2(s) {
var buf = "";
while (i < s.length) {
var ch = s[i++];
switch (ch) {
case '&':
buf += '&amp;';
break;
case '<':
buf += '&lt;';
break;
case '\"':
buf += '&quot;';
break;
default:
buf += ch;
break;
}
}
return buf;
}
function escapeHtml3(value) {
var i = 0;
var XMLChars = {
AMP: 38, // "&"
QUOT: 34, // "\""
LT: 60, // "<"
GT: 62, // ">"
};
var parts = [value.substring(0, i)];
while (i < length) {
switch (ch) {
case XMLChars.AMP:
parts.push('&amp;');
break;
case XMLChars.QUOT:
parts.push('&quot;');
break;
case XMLChars.LT:
parts.push('&lt;');
break;
case XMLChars.GT:
parts.push('&gt;');
break;
}
++i;
var j = i;
while (i < length) {
ch = value.charCodeAt(i);
if (ch === XMLChars.AMP ||
ch === XMLChars.QUOT || ch === XMLChars.LT ||
ch === XMLChars.GT) {
break;
}
i++;
}
if (j < i) {
parts.push(value.substring(j, i));
}
}
return parts.join('');
}
function escapeHtml4(s) {
var buf = "";
while (i < s.length) {
var ch = s.chatAt(i++);
switch (ch) {
case '&':
buf += '&amp;';
break;
case '<':
buf += '&lt;';
break;
case '\"':
buf += '&quot;';
break;
default:
buf += ch;
break;
}
}
return buf;
}
app.get('/user/:id', function (req, res) {
const url = req.params.id;
res.send(escapeHtml1(url)); // OK
res.send(escapeHtml2(url)); // OK
res.send(escapeHtml3(url)); // OK - but FP
res.send(escapeHtml4(url)); // OK
});

1
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXssWithCustomSanitizer.expected
View File

@@ -3,6 +3,7 @@
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | user-provided value |
| ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | Cross-site scripting vulnerability due to $@. | ReflectedXssGood3.js:135:15:135:27 | req.params.id | user-provided value |
| exception-xss.js:190:12:190:24 | req.params.id | Cross-site scripting vulnerability due to $@. | exception-xss.js:190:12:190:24 | req.params.id | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |