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

add qhelp for js/bad-code-sanitization

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-06-16 16:23:41 +02:00
parent a0951f76b6
commit 5ce17bea60
6 changed files with 110 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
View File

@@ -3,35 +3,36 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>
Placeholder
</p>
</overview>
<overview>
<p>
Using string concatenation to construct JavaScript code can be error-prone, or in the worst
case enable code-injection if an input is constructed by an attacker.
</p>
</overview>
<recommendation>
<p>
Placeholder
</p>
</recommendation>
<recommendation>
<p>
If using <code>JSON.stringify</code> or a HTML sanitizer to sanitize a string inserted into
JavaScript code, then make sure to perform additional sanitization or remove potentially
dangerous characters.
</p>
</recommendation>
<example>
<p>
Placeholder
</p>
<example>
<p>
The below example constructs a function that assigns the number 42 to the property <code>key</code>
on an object <code>obj</code>. However, if <code>key</code> contains <code>&lt;/script&gt;</code>, then
the generated code will break out of a <code>&lt;script&gt;</code> if the generated code is inserted
into a <code>&lt;script&gt;</code> tag.
</p>
<sample src="examples/ImproperCodeSanitization.js" />
<p>
The issue has been fixed in the below by escaping potentially dangerous characters.
</p>
<sample src="examples/ImproperCodeSanitizationFixed.js" />
</example>
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
</li>
<li>
MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects#Function_properties">Global functions</a>.
</li>
<li>
MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function">Function constructor</a>.
</li>
</references>
<references>
<li>OWASP: <a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.</li>
</references>
</qhelp>

4
javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitization.js Normal file
View File

@@ -0,0 +1,4 @@
function createObjectWrite() {
const assignment = `obj[${JSON.stringify(key)}]=42`;
return `(function(){${assignment}})` // NOT OK
}

23
javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitizationFixed.js Normal file
View File

@@ -0,0 +1,23 @@
const charMap = {
'<': '\\u003C',
'>' : '\\u003E',
'/': '\\u002F',
'\\': '\\\\',
'\b': '\\b',
'\f': '\\f',
'\n': '\\n',
'\r': '\\r',
'\t': '\\t',
'\0': '\\0',
'\u2028': '\\u2028',
'\u2029': '\\u2029'
};
function escapeUnsafeChars(str) {
return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x])
}
function createObjectWrite() {
const assignment = `obj[${escapeUnsafeChars(JSON.stringify(key))}]=42`;
return `(function(){${assignment}})` // OK
}

12
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
View File

@@ -34,6 +34,12 @@ nodes
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:63:11:63:55 | assignment |
| bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) |
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) |
| bad-code-sanitization.js:64:27:64:36 | assignment |
| bad-code-sanitization.js:64:27:64:36 | assignment |
edges
| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` |
@@ -53,6 +59,11 @@ edges
| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:63:11:63:55 | assignment | bad-code-sanitization.js:64:27:64:36 | assignment |
| bad-code-sanitization.js:63:11:63:55 | assignment | bad-code-sanitization.js:64:27:64:36 | assignment |
| bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` | bad-code-sanitization.js:63:11:63:55 | assignment |
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
#select
| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | $@ flows to here and is used to construct code. | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | Improperly sanitized value |
| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | Improperly sanitized value |
@@ -61,3 +72,4 @@ edges
| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | Improperly sanitized value |
| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | Improperly sanitized value |
| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | Improperly sanitized value |
| bad-code-sanitization.js:64:27:64:36 | assignment | bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | bad-code-sanitization.js:64:27:64:36 | assignment | $@ flows to here and is used to construct code. | bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | Improperly sanitized value |

33
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
View File

@@ -57,3 +57,36 @@ app.get('/some/path', function(req, res) {
setTimeout(`(function(){${JSON.stringify(taint)}))`); // OK - the source is remote-flow, and the sink is code-injection.
});
// Bad documentation example:
function createObjectWrite() {
const assignment = `obj[${JSON.stringify(key)}]=42`;
return `(function(){${assignment}})` // NOT OK
}
// Good documentation example:
function good() {
const charMap = {
'<': '\\u003C',
'>' : '\\u003E',
'/': '\\u002F',
'\\': '\\\\',
'\b': '\\b',
'\f': '\\f',
'\n': '\\n',
'\r': '\\r',
'\t': '\\t',
'\0': '\\0',
'\u2028': '\\u2028',
'\u2029': '\\u2029'
};
function escapeUnsafeChars(str) {
return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x])
}
function createObjectWrite() {
const assignment = `obj[${escapeUnsafeChars(JSON.stringify(key))}]=42`;
return `(function(){${assignment}})` // OK
}
}

9
javascript/ql/test/query-tests/Security/CWE-094/tmp.html Normal file
View File

@@ -0,0 +1,9 @@
<html>
<body>
<script>
var foo ="bla</script onload=\"\">";
</script>
</body>
</html>