Skip to content

Commit 7856e78

Browse files
codeql-cicodeql-ci
authored
Merge pull request #4566 from asgerf/js/classnames
Approved by erik-krogh
2 parents 0af62b8 + 4343fbf commit 7856e78

File tree

6 files changed

+172
-0
lines changed

6 files changed

+172
-0
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
- [debounce](https://www.npmjs.com/package/debounce)
1212
- [bluebird](https://www.npmjs.com/package/bluebird)
1313
- [call-limit](https://www.npmjs.com/package/call-limit)
14+
- [classnames](https://www.npmjs.com/package/classnames)
15+
- [clsx](https://www.npmjs.com/package/clsx)
1416
- [express](https://www.npmjs.com/package/express)
1517
- [fast-json-stable-stringify](https://www.npmjs.com/package/fast-json-stable-stringify)
1618
- [fast-safe-stringify](https://www.npmjs.com/package/fast-safe-stringify)

‎javascript/ql/src/javascript.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import semmle.javascript.frameworks.Azure
7373
import semmle.javascript.frameworks.Babel
7474
import semmle.javascript.frameworks.Cheerio
7575
import semmle.javascript.frameworks.ComposedFunctions
76+
import semmle.javascript.frameworks.Classnames
7677
import semmle.javascript.frameworks.ClientRequests
7778
import semmle.javascript.frameworks.ClosureLibrary
7879
import semmle.javascript.frameworks.CookieLibraries

‎javascript/ql/src/semmle/javascript/frameworks/Classnames.qll‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* Provides taint steps modeling flow through the `classnames` and `clsx` libraries.
3+
*/
4+
5+
import javascript
6+
7+
private DataFlow::SourceNode classnames() {
8+
result = DataFlow::moduleImport(["classnames", "classnames/dedupe", "classnames/bind"])
9+
}
10+
11+
private class PlainStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
12+
PlainStep() {
13+
this = classnames().getACall()
14+
or
15+
this = DataFlow::moduleImport("clsx").getACall()
16+
}
17+
18+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
19+
pred = getAnArgument() and
20+
succ = this
21+
}
22+
}
23+
24+
/**
25+
* Step from `x` or `y` to the result of `classnames.bind(x)(y)`.
26+
*/
27+
private class BindStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
28+
DataFlow::CallNode bind;
29+
30+
BindStep() {
31+
bind = classnames().getAMemberCall("bind") and
32+
this = bind.getACall()
33+
}
34+
35+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
36+
pred = [getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and
37+
succ = this
38+
}
39+
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,36 @@ nodes
5959
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
6060
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
6161
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
62+
| classnames.js:7:31:7:84 | `<span ... <span>` |
63+
| classnames.js:7:31:7:84 | `<span ... <span>` |
64+
| classnames.js:7:47:7:69 | classNa ... w.name) |
65+
| classnames.js:7:58:7:68 | window.name |
66+
| classnames.js:7:58:7:68 | window.name |
67+
| classnames.js:8:31:8:85 | `<span ... <span>` |
68+
| classnames.js:8:31:8:85 | `<span ... <span>` |
69+
| classnames.js:8:47:8:70 | classNa ... w.name) |
70+
| classnames.js:8:59:8:69 | window.name |
71+
| classnames.js:8:59:8:69 | window.name |
72+
| classnames.js:9:31:9:85 | `<span ... <span>` |
73+
| classnames.js:9:31:9:85 | `<span ... <span>` |
74+
| classnames.js:9:47:9:70 | classNa ... w.name) |
75+
| classnames.js:9:59:9:69 | window.name |
76+
| classnames.js:9:59:9:69 | window.name |
77+
| classnames.js:10:45:10:55 | window.name |
78+
| classnames.js:10:45:10:55 | window.name |
79+
| classnames.js:11:31:11:79 | `<span ... <span>` |
80+
| classnames.js:11:31:11:79 | `<span ... <span>` |
81+
| classnames.js:11:47:11:64 | unsafeStyle('foo') |
82+
| classnames.js:13:31:13:83 | `<span ... <span>` |
83+
| classnames.js:13:31:13:83 | `<span ... <span>` |
84+
| classnames.js:13:47:13:68 | safeSty ... w.name) |
85+
| classnames.js:13:57:13:67 | window.name |
86+
| classnames.js:13:57:13:67 | window.name |
87+
| classnames.js:15:31:15:78 | `<span ... <span>` |
88+
| classnames.js:15:31:15:78 | `<span ... <span>` |
89+
| classnames.js:15:47:15:63 | clsx(window.name) |
90+
| classnames.js:15:52:15:62 | window.name |
91+
| classnames.js:15:52:15:62 | window.name |
6292
| jquery.js:2:7:2:40 | tainted |
6393
| jquery.js:2:7:2:40 | tainted |
6494
| jquery.js:2:17:2:33 | document.location |
@@ -601,6 +631,30 @@ edges
601631
| angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x |
602632
| angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url |
603633
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
634+
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
635+
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
636+
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |
637+
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |
638+
| classnames.js:8:47:8:70 | classNa ... w.name) | classnames.js:8:31:8:85 | `<span ... <span>` |
639+
| classnames.js:8:47:8:70 | classNa ... w.name) | classnames.js:8:31:8:85 | `<span ... <span>` |
640+
| classnames.js:8:59:8:69 | window.name | classnames.js:8:47:8:70 | classNa ... w.name) |
641+
| classnames.js:8:59:8:69 | window.name | classnames.js:8:47:8:70 | classNa ... w.name) |
642+
| classnames.js:9:47:9:70 | classNa ... w.name) | classnames.js:9:31:9:85 | `<span ... <span>` |
643+
| classnames.js:9:47:9:70 | classNa ... w.name) | classnames.js:9:31:9:85 | `<span ... <span>` |
644+
| classnames.js:9:59:9:69 | window.name | classnames.js:9:47:9:70 | classNa ... w.name) |
645+
| classnames.js:9:59:9:69 | window.name | classnames.js:9:47:9:70 | classNa ... w.name) |
646+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:47:11:64 | unsafeStyle('foo') |
647+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:47:11:64 | unsafeStyle('foo') |
648+
| classnames.js:11:47:11:64 | unsafeStyle('foo') | classnames.js:11:31:11:79 | `<span ... <span>` |
649+
| classnames.js:11:47:11:64 | unsafeStyle('foo') | classnames.js:11:31:11:79 | `<span ... <span>` |
650+
| classnames.js:13:47:13:68 | safeSty ... w.name) | classnames.js:13:31:13:83 | `<span ... <span>` |
651+
| classnames.js:13:47:13:68 | safeSty ... w.name) | classnames.js:13:31:13:83 | `<span ... <span>` |
652+
| classnames.js:13:57:13:67 | window.name | classnames.js:13:47:13:68 | safeSty ... w.name) |
653+
| classnames.js:13:57:13:67 | window.name | classnames.js:13:47:13:68 | safeSty ... w.name) |
654+
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
655+
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
656+
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
657+
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
604658
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
605659
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
606660
| jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search |
@@ -1063,6 +1117,12 @@ edges
10631117
| angular2-client.ts:35:44:35:91 | this.ro ... arams.x | angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x | Cross-site scripting vulnerability due to $@. | angular2-client.ts:35:44:35:89 | this.ro ... .params | user-provided value |
10641118
| angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | Cross-site scripting vulnerability due to $@. | angular2-client.ts:37:44:37:58 | this.router.url | user-provided value |
10651119
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | Cross-site scripting vulnerability due to $@. | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | user-provided value |
1120+
| classnames.js:7:31:7:84 | `<span ... <span>` | classnames.js:7:58:7:68 | window.name | classnames.js:7:31:7:84 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:7:58:7:68 | window.name | user-provided value |
1121+
| classnames.js:8:31:8:85 | `<span ... <span>` | classnames.js:8:59:8:69 | window.name | classnames.js:8:31:8:85 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:8:59:8:69 | window.name | user-provided value |
1122+
| classnames.js:9:31:9:85 | `<span ... <span>` | classnames.js:9:59:9:69 | window.name | classnames.js:9:31:9:85 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:9:59:9:69 | window.name | user-provided value |
1123+
| classnames.js:11:31:11:79 | `<span ... <span>` | classnames.js:10:45:10:55 | window.name | classnames.js:11:31:11:79 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:10:45:10:55 | window.name | user-provided value |
1124+
| classnames.js:13:31:13:83 | `<span ... <span>` | classnames.js:13:57:13:67 | window.name | classnames.js:13:31:13:83 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:13:57:13:67 | window.name | user-provided value |
1125+
| classnames.js:15:31:15:78 | `<span ... <span>` | classnames.js:15:52:15:62 | window.name | classnames.js:15:31:15:78 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:15:52:15:62 | window.name | user-provided value |
10661126
| jquery.js:7:5:7:34 | "<div i ... + "\\">" | jquery.js:2:17:2:40 | documen ... .search | jquery.js:7:5:7:34 | "<div i ... + "\\">" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
10671127
| jquery.js:8:18:8:34 | "XSS: " + tainted | jquery.js:2:17:2:33 | document.location | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
10681128
| jquery.js:10:5:10:40 | "<b>" + ... "</b>" | jquery.js:10:13:10:20 | location | jquery.js:10:5:10:40 | "<b>" + ... "</b>" | Cross-site scripting vulnerability due to $@. | jquery.js:10:13:10:20 | location | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,36 @@ nodes
5959
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
6060
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
6161
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
62+
| classnames.js:7:31:7:84 | `<span ... <span>` |
63+
| classnames.js:7:31:7:84 | `<span ... <span>` |
64+
| classnames.js:7:47:7:69 | classNa ... w.name) |
65+
| classnames.js:7:58:7:68 | window.name |
66+
| classnames.js:7:58:7:68 | window.name |
67+
| classnames.js:8:31:8:85 | `<span ... <span>` |
68+
| classnames.js:8:31:8:85 | `<span ... <span>` |
69+
| classnames.js:8:47:8:70 | classNa ... w.name) |
70+
| classnames.js:8:59:8:69 | window.name |
71+
| classnames.js:8:59:8:69 | window.name |
72+
| classnames.js:9:31:9:85 | `<span ... <span>` |
73+
| classnames.js:9:31:9:85 | `<span ... <span>` |
74+
| classnames.js:9:47:9:70 | classNa ... w.name) |
75+
| classnames.js:9:59:9:69 | window.name |
76+
| classnames.js:9:59:9:69 | window.name |
77+
| classnames.js:10:45:10:55 | window.name |
78+
| classnames.js:10:45:10:55 | window.name |
79+
| classnames.js:11:31:11:79 | `<span ... <span>` |
80+
| classnames.js:11:31:11:79 | `<span ... <span>` |
81+
| classnames.js:11:47:11:64 | unsafeStyle('foo') |
82+
| classnames.js:13:31:13:83 | `<span ... <span>` |
83+
| classnames.js:13:31:13:83 | `<span ... <span>` |
84+
| classnames.js:13:47:13:68 | safeSty ... w.name) |
85+
| classnames.js:13:57:13:67 | window.name |
86+
| classnames.js:13:57:13:67 | window.name |
87+
| classnames.js:15:31:15:78 | `<span ... <span>` |
88+
| classnames.js:15:31:15:78 | `<span ... <span>` |
89+
| classnames.js:15:47:15:63 | clsx(window.name) |
90+
| classnames.js:15:52:15:62 | window.name |
91+
| classnames.js:15:52:15:62 | window.name |
6292
| jquery.js:2:7:2:40 | tainted |
6393
| jquery.js:2:7:2:40 | tainted |
6494
| jquery.js:2:17:2:33 | document.location |
@@ -605,6 +635,30 @@ edges
605635
| angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x |
606636
| angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url |
607637
| angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') |
638+
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
639+
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
640+
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |
641+
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |
642+
| classnames.js:8:47:8:70 | classNa ... w.name) | classnames.js:8:31:8:85 | `<span ... <span>` |
643+
| classnames.js:8:47:8:70 | classNa ... w.name) | classnames.js:8:31:8:85 | `<span ... <span>` |
644+
| classnames.js:8:59:8:69 | window.name | classnames.js:8:47:8:70 | classNa ... w.name) |
645+
| classnames.js:8:59:8:69 | window.name | classnames.js:8:47:8:70 | classNa ... w.name) |
646+
| classnames.js:9:47:9:70 | classNa ... w.name) | classnames.js:9:31:9:85 | `<span ... <span>` |
647+
| classnames.js:9:47:9:70 | classNa ... w.name) | classnames.js:9:31:9:85 | `<span ... <span>` |
648+
| classnames.js:9:59:9:69 | window.name | classnames.js:9:47:9:70 | classNa ... w.name) |
649+
| classnames.js:9:59:9:69 | window.name | classnames.js:9:47:9:70 | classNa ... w.name) |
650+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:47:11:64 | unsafeStyle('foo') |
651+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:47:11:64 | unsafeStyle('foo') |
652+
| classnames.js:11:47:11:64 | unsafeStyle('foo') | classnames.js:11:31:11:79 | `<span ... <span>` |
653+
| classnames.js:11:47:11:64 | unsafeStyle('foo') | classnames.js:11:31:11:79 | `<span ... <span>` |
654+
| classnames.js:13:47:13:68 | safeSty ... w.name) | classnames.js:13:31:13:83 | `<span ... <span>` |
655+
| classnames.js:13:47:13:68 | safeSty ... w.name) | classnames.js:13:31:13:83 | `<span ... <span>` |
656+
| classnames.js:13:57:13:67 | window.name | classnames.js:13:47:13:68 | safeSty ... w.name) |
657+
| classnames.js:13:57:13:67 | window.name | classnames.js:13:47:13:68 | safeSty ... w.name) |
658+
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
659+
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
660+
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
661+
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
608662
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
609663
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
610664
| jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/classnames.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import classNames from 'classnames';
2+
import classNamesD from 'classnames/dedupe';
3+
import classNamesB from 'classnames/bind';
4+
import clsx from 'clsx';
5+
6+
function main() {
7+
document.body.innerHTML = `<span class="${classNames(window.name)}">Hello<span>`; // NOT OK
8+
document.body.innerHTML = `<span class="${classNamesD(window.name)}">Hello<span>`; // NOT OK
9+
document.body.innerHTML = `<span class="${classNamesB(window.name)}">Hello<span>`; // NOT OK
10+
let unsafeStyle = classNames.bind({foo: window.name});
11+
document.body.innerHTML = `<span class="${unsafeStyle('foo')}">Hello<span>`; // NOT OK
12+
let safeStyle = classNames.bind({});
13+
document.body.innerHTML = `<span class="${safeStyle(window.name)}">Hello<span>`; // NOT OK
14+
document.body.innerHTML = `<span class="${safeStyle('foo')}">Hello<span>`; // OK
15+
document.body.innerHTML = `<span class="${clsx(window.name)}">Hello<span>`; // NOT OK
16+
}

0 commit comments

Comments
 (0)