Merge branch 'js-team-sprint' into priv-file-polish

This commit is contained in:
Erik Krogh Kristensen 2020-06-17 21:03:09 +02:00
Родитель b0be0eb805 6d6f29eb85
Коммит 73f26956a6
10 изменённых файлов: 170 добавлений и 35 удалений

Просмотреть файл

@ -37,6 +37,7 @@
| Unsafe expansion of self-closing HTML tag (`js/unsafe-html-expansion`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities caused by unsafe expansion of self-closing HTML tags. |
| Unsafe shell command constructed from library input (`js/shell-command-constructed-from-input`) | correctness, security, external/cwe/cwe-078, external/cwe/cwe-088 | Highlights potential command injections due to a shell command being constructed from library inputs. Results are shown on LGTM by default. |
| Exposure of private files (`js/exposure-of-private-files`) | security, external/cwe/cwe-200 | Highlights servers that serve private files. Results are shown on LGTM by default. |
| Improper code sanitization (`js/bad-code-sanitization`) | security, external/cwe/cwe-094, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights string concatenation where code is constructed without proper sanitization. Results are shown on LGTM by default. |
## Changes to existing queries

Просмотреть файл

@ -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 example below 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 inserted into a
<code>&lt;/script&gt;</code> tag.
</p>
<sample src="examples/ImproperCodeSanitization.js" />
<p>
The issue has been fixed by escaping potentially dangerous characters, as shown below.
</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>

Просмотреть файл

@ -1,6 +1,6 @@
/**
* @name Improper code sanitization
* @description Escaping code as HTML does not provide protection against code-injection.
* @description Escaping code as HTML does not provide protection against code injection.
* @kind path-problem
* @problem.severity error
* @precision high
@ -20,20 +20,25 @@ private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
/**
* Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`.
*/
private DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
private DataFlow::Node remoteFlow(DataFlow::TypeTracker t) {
t.start() and
result instanceof RemoteFlowSource
or
exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t))
exists(DataFlow::TypeTracker t2, DataFlow::Node prev | prev = remoteFlow(t2) |
t2 = t.smallstep(prev, result)
or
any(TaintTracking::AdditionalTaintStep dts).step(prev, result) and
t = t2
)
}
/**
* Gets a type-tracked reference to a `RemoteFlowSource`.
*/
private DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
private DataFlow::Node remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
/**
* Gets a type-back-tracked instance of a code-injection sink using type-tracker `t`.
* Gets a type-back-tracked instance of a code injection sink using type-tracker `t`.
*/
private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
t.start() and
@ -48,7 +53,7 @@ private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
}
/**
* Gets a reference to to a data-flow node that ends in a code-injection sink.
* Gets a reference to to a data-flow node that ends in a code injection sink.
*/
private DataFlow::Node endsInCodeInjectionSink() {
result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
@ -60,7 +65,7 @@ where
// Basic detection of duplicate results with `js/code-injection`.
not (
sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and
remoteFlow().flowsTo(source.getNode().(DataFlow::InvokeNode).getAnArgument())
remoteFlow() = source.getNode().(DataFlow::InvokeNode).getAnArgument()
)
select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
source.getNode(), "Improperly sanitized value"

Просмотреть файл

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

Просмотреть файл

@ -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
}

Просмотреть файл

@ -69,6 +69,16 @@ nodes
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
| bad-code-sanitization.js:56:7:56:47 | taint |
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
| bad-code-sanitization.js:56:16:56:23 | req.body |
| bad-code-sanitization.js:56:16:56:23 | req.body |
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:44:58:48 | taint |
| express.js:7:24:7:69 | "return ... + "];" |
| express.js:7:24:7:69 | "return ... + "];" |
| express.js:7:44:7:62 | req.param("wobble") |
@ -202,6 +212,15 @@ edges
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
@ -271,6 +290,7 @@ edges
| angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:29 | location | User-provided value |
| angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:39 | location | User-provided value |
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | User-provided value |
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` | bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:56:16:56:23 | req.body | User-provided value |
| express.js:7:24:7:69 | "return ... + "];" | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value |
| express.js:9:34:9:79 | "return ... + "];" | express.js:9:54:9:72 | req.param("wobble") | express.js:9:34:9:79 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value |
| express.js:12:8:12:53 | "return ... + "];" | express.js:12:28:12:46 | req.param("wobble") | express.js:12:8:12:53 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:12:28:12:46 | req.param("wobble") | User-provided value |

Просмотреть файл

@ -69,6 +69,16 @@ nodes
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
| bad-code-sanitization.js:56:7:56:47 | taint |
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
| bad-code-sanitization.js:56:16:56:23 | req.body |
| bad-code-sanitization.js:56:16:56:23 | req.body |
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:44:58:48 | taint |
| eslint-escope-build.js:20:22:20:22 | c |
| eslint-escope-build.js:20:22:20:22 | c |
| eslint-escope-build.js:21:16:21:16 | c |
@ -206,6 +216,15 @@ edges
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |

Просмотреть файл

@ -31,6 +31,15 @@ nodes
| 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: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: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)}]` |
@ -49,6 +58,12 @@ edges
| 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: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 |
@ -57,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 |

Просмотреть файл

@ -52,4 +52,41 @@ app.get('/some/path', function(req, res) {
var foo = `(function(){${JSON.stringify(req.param("wobble"))}))` // NOT - the source is remote-flow, but we know of no sink.
setTimeout(`(function(){${JSON.stringify(req.param("wobble"))}))`); // OK - the source is remote-flow, and the sink is code-injection.
var taint = [req.body.name, "foo"].join("\n");
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
}
}

Просмотреть файл

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