Skip to content

Commit 6d6f29e

Browse files
erik-krogherik-krogh
authored
Merge pull request github#3726 from erik-krogh/bad-code-polish
JS: Bad code polish
2 parents ac1a0d9 + a465fef commit 6d6f29e

File tree

10 files changed

+170
-35
lines changed

10 files changed

+170
-35
lines changed

‎change-notes/1.25/analysis-javascript.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
| Incomplete HTML attribute sanitization (`js/incomplete-html-attribute-sanitization`) | security, external/cwe/cwe-20, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities due to incomplete sanitization of HTML meta-characters. Results are shown on LGTM by default. |
3737
| 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. |
3838
| 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. |
39+
| 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. |
3940

4041
## Changes to existing queries
4142

‎javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,36 @@
33
"qhelp.dtd">
44
<qhelp>
55

6-
<overview>
7-
<p>
8-
Placeholder
9-
</p>
10-
</overview>
6+
<overview>
7+
<p>
8+
Using string concatenation to construct JavaScript code can be error-prone, or in the worst
9+
case, enable code injection if an input is constructed by an attacker.
10+
</p>
11+
</overview>
1112

12-
<recommendation>
13-
<p>
14-
Placeholder
15-
</p>
16-
</recommendation>
13+
<recommendation>
14+
<p>
15+
If using <code>JSON.stringify</code> or a HTML sanitizer to sanitize a string inserted into
16+
JavaScript code, then make sure to perform additional sanitization or remove potentially
17+
dangerous characters.
18+
</p>
19+
</recommendation>
1720

18-
<example>
19-
<p>
20-
Placeholder
21-
</p>
21+
<example>
22+
<p>
23+
The example below constructs a function that assigns the number 42 to the property <code>key</code>
24+
on an object <code>obj</code>. However, if <code>key</code> contains <code>&lt;/script&gt;</code>, then
25+
the generated code will break out of a <code>&lt;/script&gt;</code> if inserted into a
26+
<code>&lt;/script&gt;</code> tag.
27+
</p>
28+
<sample src="examples/ImproperCodeSanitization.js" />
29+
<p>
30+
The issue has been fixed by escaping potentially dangerous characters, as shown below.
31+
</p>
32+
<sample src="examples/ImproperCodeSanitizationFixed.js" />
33+
</example>
2234

23-
</example>
24-
25-
<references>
26-
<li>
27-
OWASP:
28-
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
29-
</li>
30-
<li>
31-
MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects#Function_properties">Global functions</a>.
32-
</li>
33-
<li>
34-
MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function">Function constructor</a>.
35-
</li>
36-
</references>
35+
<references>
36+
<li>OWASP: <a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.</li>
37+
</references>
3738
</qhelp>

‎javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name Improper code sanitization
3-
* @description Escaping code as HTML does not provide protection against code-injection.
3+
* @description Escaping code as HTML does not provide protection against code injection.
44
* @kind path-problem
55
* @problem.severity error
66
* @precision high
@@ -20,20 +20,25 @@ private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
2020
/**
2121
* Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`.
2222
*/
23-
private DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
23+
private DataFlow::Node remoteFlow(DataFlow::TypeTracker t) {
2424
t.start() and
2525
result instanceof RemoteFlowSource
2626
or
27-
exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t))
27+
exists(DataFlow::TypeTracker t2, DataFlow::Node prev | prev = remoteFlow(t2) |
28+
t2 = t.smallstep(prev, result)
29+
or
30+
any(TaintTracking::AdditionalTaintStep dts).step(prev, result) and
31+
t = t2
32+
)
2833
}
2934

3035
/**
3136
* Gets a type-tracked reference to a `RemoteFlowSource`.
3237
*/
33-
private DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
38+
private DataFlow::Node remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
3439

3540
/**
36-
* Gets a type-back-tracked instance of a code-injection sink using type-tracker `t`.
41+
* Gets a type-back-tracked instance of a code injection sink using type-tracker `t`.
3742
*/
3843
private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
3944
t.start() and
@@ -48,7 +53,7 @@ private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
4853
}
4954

5055
/**
51-
* Gets a reference to to a data-flow node that ends in a code-injection sink.
56+
* Gets a reference to to a data-flow node that ends in a code injection sink.
5257
*/
5358
private DataFlow::Node endsInCodeInjectionSink() {
5459
result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
@@ -60,7 +65,7 @@ where
6065
// Basic detection of duplicate results with `js/code-injection`.
6166
not (
6267
sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and
63-
remoteFlow().flowsTo(source.getNode().(DataFlow::InvokeNode).getAnArgument())
68+
remoteFlow() = source.getNode().(DataFlow::InvokeNode).getAnArgument()
6469
)
6570
select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
6671
source.getNode(), "Improperly sanitized value"

‎javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitization.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
function createObjectWrite() {
2+
const assignment = `obj[${JSON.stringify(key)}]=42`;
3+
return `(function(){${assignment}})` // NOT OK
4+
}

‎javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitizationFixed.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const charMap = {
2+
'<': '\\u003C',
3+
'>' : '\\u003E',
4+
'/': '\\u002F',
5+
'\\': '\\\\',
6+
'\b': '\\b',
7+
'\f': '\\f',
8+
'\n': '\\n',
9+
'\r': '\\r',
10+
'\t': '\\t',
11+
'\0': '\\0',
12+
'\u2028': '\\u2028',
13+
'\u2029': '\\u2029'
14+
};
15+
16+
function escapeUnsafeChars(str) {
17+
return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x])
18+
}
19+
20+
function createObjectWrite() {
21+
const assignment = `obj[${escapeUnsafeChars(JSON.stringify(key))}]=42`;
22+
return `(function(){${assignment}})` // OK
23+
}

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ nodes
6969
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
7070
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
7171
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
72+
| bad-code-sanitization.js:56:7:56:47 | taint |
73+
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
74+
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
75+
| bad-code-sanitization.js:56:16:56:23 | req.body |
76+
| bad-code-sanitization.js:56:16:56:23 | req.body |
77+
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
78+
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
79+
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
80+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
81+
| bad-code-sanitization.js:58:44:58:48 | taint |
7282
| express.js:7:24:7:69 | "return ... + "];" |
7383
| express.js:7:24:7:69 | "return ... + "];" |
7484
| express.js:7:44:7:62 | req.param("wobble") |
@@ -202,6 +212,15 @@ edges
202212
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
203213
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
204214
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
215+
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
216+
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
217+
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
218+
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
219+
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
220+
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
221+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
222+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
223+
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
205224
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
206225
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
207226
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
@@ -271,6 +290,7 @@ edges
271290
| 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 |
272291
| 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 |
273292
| 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 |
293+
| 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 |
274294
| 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 |
275295
| 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 |
276296
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ nodes
6969
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
7070
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
7171
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
72+
| bad-code-sanitization.js:56:7:56:47 | taint |
73+
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
74+
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
75+
| bad-code-sanitization.js:56:16:56:23 | req.body |
76+
| bad-code-sanitization.js:56:16:56:23 | req.body |
77+
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
78+
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
79+
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
80+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
81+
| bad-code-sanitization.js:58:44:58:48 | taint |
7282
| eslint-escope-build.js:20:22:20:22 | c |
7383
| eslint-escope-build.js:20:22:20:22 | c |
7484
| eslint-escope-build.js:21:16:21:16 | c |
@@ -206,6 +216,15 @@ edges
206216
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
207217
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
208218
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
219+
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
220+
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
221+
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
222+
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
223+
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
224+
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
225+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
226+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
227+
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
209228
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
210229
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
211230
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ nodes
3131
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
3232
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
3333
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
34+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
35+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
36+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
37+
| bad-code-sanitization.js:63:11:63:55 | assignment |
38+
| bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
39+
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) |
40+
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) |
41+
| bad-code-sanitization.js:64:27:64:36 | assignment |
42+
| bad-code-sanitization.js:64:27:64:36 | assignment |
3443
edges
3544
| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
3645
| 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
4958
| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
5059
| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
5160
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
61+
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
62+
| bad-code-sanitization.js:63:11:63:55 | assignment | bad-code-sanitization.js:64:27:64:36 | assignment |
63+
| bad-code-sanitization.js:63:11:63:55 | assignment | bad-code-sanitization.js:64:27:64:36 | assignment |
64+
| bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` | bad-code-sanitization.js:63:11:63:55 | assignment |
65+
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
66+
| bad-code-sanitization.js:63:31:63:49 | JSON.stringify(key) | bad-code-sanitization.js:63:24:63:55 | `obj[${ ... )}]=42` |
5267
#select
5368
| 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 |
5469
| 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
5772
| 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 |
5873
| 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 |
5974
| 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 |
75+
| 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 |

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,41 @@ app.get('/some/path', function(req, res) {
5252
var foo = `(function(){${JSON.stringify(req.param("wobble"))}))` // NOT - the source is remote-flow, but we know of no sink.
5353

5454
setTimeout(`(function(){${JSON.stringify(req.param("wobble"))}))`); // OK - the source is remote-flow, and the sink is code-injection.
55+
56+
var taint = [req.body.name, "foo"].join("\n");
57+
58+
setTimeout(`(function(){${JSON.stringify(taint)}))`); // OK - the source is remote-flow, and the sink is code-injection.
5559
});
60+
61+
// Bad documentation example:
62+
function createObjectWrite() {
63+
const assignment = `obj[${JSON.stringify(key)}]=42`;
64+
return `(function(){${assignment}})` // NOT OK
65+
}
66+
67+
// Good documentation example:
68+
function good() {
69+
const charMap = {
70+
'<': '\\u003C',
71+
'>' : '\\u003E',
72+
'/': '\\u002F',
73+
'\\': '\\\\',
74+
'\b': '\\b',
75+
'\f': '\\f',
76+
'\n': '\\n',
77+
'\r': '\\r',
78+
'\t': '\\t',
79+
'\0': '\\0',
80+
'\u2028': '\\u2028',
81+
'\u2029': '\\u2029'
82+
};
83+
84+
function escapeUnsafeChars(str) {
85+
return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x])
86+
}
87+
88+
function createObjectWrite() {
89+
const assignment = `obj[${escapeUnsafeChars(JSON.stringify(key))}]=42`;
90+
return `(function(){${assignment}})` // OK
91+
}
92+
}

‎javascript/ql/test/query-tests/Security/CWE-094/tmp.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<html>
2+
3+
<body>
4+
<script>
5+
var foo ="bla</script onload=\"\">";
6+
</script>
7+
</body>
8+
9+
</html>

0 commit comments

Comments
 (0)