Skip to content

Commit 3d86e85

Browse files
committed
JS: Add model of classnames and clsx
1 parent 7993a83 commit 3d86e85

File tree

5 files changed

+169
-0
lines changed

5 files changed

+169
-0
lines changed

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

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:82 | `<span ... <span>` |
63+
| classnames.js:7:31:7:82 | `<span ... <span>` |
64+
| classnames.js:7:46:7:68 | classNa ... w.name) |
65+
| classnames.js:7:57:7:67 | window.name |
66+
| classnames.js:7:57:7:67 | window.name |
67+
| classnames.js:8:31:8:83 | `<span ... <span>` |
68+
| classnames.js:8:31:8:83 | `<span ... <span>` |
69+
| classnames.js:8:46:8:69 | classNa ... w.name) |
70+
| classnames.js:8:58:8:68 | window.name |
71+
| classnames.js:8:58:8:68 | window.name |
72+
| classnames.js:9:31:9:83 | `<span ... <span>` |
73+
| classnames.js:9:31:9:83 | `<span ... <span>` |
74+
| classnames.js:9:46:9:69 | classNa ... w.name) |
75+
| classnames.js:9:58:9:68 | window.name |
76+
| classnames.js:9:58:9:68 | 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:77 | `<span ... <span>` |
80+
| classnames.js:11:31:11:77 | `<span ... <span>` |
81+
| classnames.js:11:46:11:63 | unsafeStyle('foo') |
82+
| classnames.js:13:31:13:81 | `<span ... <span>` |
83+
| classnames.js:13:31:13:81 | `<span ... <span>` |
84+
| classnames.js:13:46:13:67 | safeSty ... w.name) |
85+
| classnames.js:13:56:13:66 | window.name |
86+
| classnames.js:13:56:13:66 | window.name |
87+
| classnames.js:15:31:15:76 | `<span ... <span>` |
88+
| classnames.js:15:31:15:76 | `<span ... <span>` |
89+
| classnames.js:15:46:15:62 | clsx(window.name) |
90+
| classnames.js:15:51:15:61 | window.name |
91+
| classnames.js:15:51:15:61 | 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:46:7:68 | classNa ... w.name) | classnames.js:7:31:7:82 | `<span ... <span>` |
635+
| classnames.js:7:46:7:68 | classNa ... w.name) | classnames.js:7:31:7:82 | `<span ... <span>` |
636+
| classnames.js:7:57:7:67 | window.name | classnames.js:7:46:7:68 | classNa ... w.name) |
637+
| classnames.js:7:57:7:67 | window.name | classnames.js:7:46:7:68 | classNa ... w.name) |
638+
| classnames.js:8:46:8:69 | classNa ... w.name) | classnames.js:8:31:8:83 | `<span ... <span>` |
639+
| classnames.js:8:46:8:69 | classNa ... w.name) | classnames.js:8:31:8:83 | `<span ... <span>` |
640+
| classnames.js:8:58:8:68 | window.name | classnames.js:8:46:8:69 | classNa ... w.name) |
641+
| classnames.js:8:58:8:68 | window.name | classnames.js:8:46:8:69 | classNa ... w.name) |
642+
| classnames.js:9:46:9:69 | classNa ... w.name) | classnames.js:9:31:9:83 | `<span ... <span>` |
643+
| classnames.js:9:46:9:69 | classNa ... w.name) | classnames.js:9:31:9:83 | `<span ... <span>` |
644+
| classnames.js:9:58:9:68 | window.name | classnames.js:9:46:9:69 | classNa ... w.name) |
645+
| classnames.js:9:58:9:68 | window.name | classnames.js:9:46:9:69 | classNa ... w.name) |
646+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:46:11:63 | unsafeStyle('foo') |
647+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:46:11:63 | unsafeStyle('foo') |
648+
| classnames.js:11:46:11:63 | unsafeStyle('foo') | classnames.js:11:31:11:77 | `<span ... <span>` |
649+
| classnames.js:11:46:11:63 | unsafeStyle('foo') | classnames.js:11:31:11:77 | `<span ... <span>` |
650+
| classnames.js:13:46:13:67 | safeSty ... w.name) | classnames.js:13:31:13:81 | `<span ... <span>` |
651+
| classnames.js:13:46:13:67 | safeSty ... w.name) | classnames.js:13:31:13:81 | `<span ... <span>` |
652+
| classnames.js:13:56:13:66 | window.name | classnames.js:13:46:13:67 | safeSty ... w.name) |
653+
| classnames.js:13:56:13:66 | window.name | classnames.js:13:46:13:67 | safeSty ... w.name) |
654+
| classnames.js:15:46:15:62 | clsx(window.name) | classnames.js:15:31:15:76 | `<span ... <span>` |
655+
| classnames.js:15:46:15:62 | clsx(window.name) | classnames.js:15:31:15:76 | `<span ... <span>` |
656+
| classnames.js:15:51:15:61 | window.name | classnames.js:15:46:15:62 | clsx(window.name) |
657+
| classnames.js:15:51:15:61 | window.name | classnames.js:15:46:15:62 | 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:82 | `<span ... <span>` | classnames.js:7:57:7:67 | window.name | classnames.js:7:31:7:82 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:7:57:7:67 | window.name | user-provided value |
1121+
| classnames.js:8:31:8:83 | `<span ... <span>` | classnames.js:8:58:8:68 | window.name | classnames.js:8:31:8:83 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:8:58:8:68 | window.name | user-provided value |
1122+
| classnames.js:9:31:9:83 | `<span ... <span>` | classnames.js:9:58:9:68 | window.name | classnames.js:9:31:9:83 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:9:58:9:68 | window.name | user-provided value |
1123+
| classnames.js:11:31:11:77 | `<span ... <span>` | classnames.js:10:45:10:55 | window.name | classnames.js:11:31:11:77 | `<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:81 | `<span ... <span>` | classnames.js:13:56:13:66 | window.name | classnames.js:13:31:13:81 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:13:56:13:66 | window.name | user-provided value |
1125+
| classnames.js:15:31:15:76 | `<span ... <span>` | classnames.js:15:51:15:61 | window.name | classnames.js:15:31:15:76 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:15:51:15:61 | 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:82 | `<span ... <span>` |
63+
| classnames.js:7:31:7:82 | `<span ... <span>` |
64+
| classnames.js:7:46:7:68 | classNa ... w.name) |
65+
| classnames.js:7:57:7:67 | window.name |
66+
| classnames.js:7:57:7:67 | window.name |
67+
| classnames.js:8:31:8:83 | `<span ... <span>` |
68+
| classnames.js:8:31:8:83 | `<span ... <span>` |
69+
| classnames.js:8:46:8:69 | classNa ... w.name) |
70+
| classnames.js:8:58:8:68 | window.name |
71+
| classnames.js:8:58:8:68 | window.name |
72+
| classnames.js:9:31:9:83 | `<span ... <span>` |
73+
| classnames.js:9:31:9:83 | `<span ... <span>` |
74+
| classnames.js:9:46:9:69 | classNa ... w.name) |
75+
| classnames.js:9:58:9:68 | window.name |
76+
| classnames.js:9:58:9:68 | 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:77 | `<span ... <span>` |
80+
| classnames.js:11:31:11:77 | `<span ... <span>` |
81+
| classnames.js:11:46:11:63 | unsafeStyle('foo') |
82+
| classnames.js:13:31:13:81 | `<span ... <span>` |
83+
| classnames.js:13:31:13:81 | `<span ... <span>` |
84+
| classnames.js:13:46:13:67 | safeSty ... w.name) |
85+
| classnames.js:13:56:13:66 | window.name |
86+
| classnames.js:13:56:13:66 | window.name |
87+
| classnames.js:15:31:15:76 | `<span ... <span>` |
88+
| classnames.js:15:31:15:76 | `<span ... <span>` |
89+
| classnames.js:15:46:15:62 | clsx(window.name) |
90+
| classnames.js:15:51:15:61 | window.name |
91+
| classnames.js:15:51:15:61 | 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:46:7:68 | classNa ... w.name) | classnames.js:7:31:7:82 | `<span ... <span>` |
639+
| classnames.js:7:46:7:68 | classNa ... w.name) | classnames.js:7:31:7:82 | `<span ... <span>` |
640+
| classnames.js:7:57:7:67 | window.name | classnames.js:7:46:7:68 | classNa ... w.name) |
641+
| classnames.js:7:57:7:67 | window.name | classnames.js:7:46:7:68 | classNa ... w.name) |
642+
| classnames.js:8:46:8:69 | classNa ... w.name) | classnames.js:8:31:8:83 | `<span ... <span>` |
643+
| classnames.js:8:46:8:69 | classNa ... w.name) | classnames.js:8:31:8:83 | `<span ... <span>` |
644+
| classnames.js:8:58:8:68 | window.name | classnames.js:8:46:8:69 | classNa ... w.name) |
645+
| classnames.js:8:58:8:68 | window.name | classnames.js:8:46:8:69 | classNa ... w.name) |
646+
| classnames.js:9:46:9:69 | classNa ... w.name) | classnames.js:9:31:9:83 | `<span ... <span>` |
647+
| classnames.js:9:46:9:69 | classNa ... w.name) | classnames.js:9:31:9:83 | `<span ... <span>` |
648+
| classnames.js:9:58:9:68 | window.name | classnames.js:9:46:9:69 | classNa ... w.name) |
649+
| classnames.js:9:58:9:68 | window.name | classnames.js:9:46:9:69 | classNa ... w.name) |
650+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:46:11:63 | unsafeStyle('foo') |
651+
| classnames.js:10:45:10:55 | window.name | classnames.js:11:46:11:63 | unsafeStyle('foo') |
652+
| classnames.js:11:46:11:63 | unsafeStyle('foo') | classnames.js:11:31:11:77 | `<span ... <span>` |
653+
| classnames.js:11:46:11:63 | unsafeStyle('foo') | classnames.js:11:31:11:77 | `<span ... <span>` |
654+
| classnames.js:13:46:13:67 | safeSty ... w.name) | classnames.js:13:31:13:81 | `<span ... <span>` |
655+
| classnames.js:13:46:13:67 | safeSty ... w.name) | classnames.js:13:31:13:81 | `<span ... <span>` |
656+
| classnames.js:13:56:13:66 | window.name | classnames.js:13:46:13:67 | safeSty ... w.name) |
657+
| classnames.js:13:56:13:66 | window.name | classnames.js:13:46:13:67 | safeSty ... w.name) |
658+
| classnames.js:15:46:15:62 | clsx(window.name) | classnames.js:15:31:15:76 | `<span ... <span>` |
659+
| classnames.js:15:46:15:62 | clsx(window.name) | classnames.js:15:31:15:76 | `<span ... <span>` |
660+
| classnames.js:15:51:15:61 | window.name | classnames.js:15:46:15:62 | clsx(window.name) |
661+
| classnames.js:15:51:15:61 | window.name | classnames.js:15:46:15:62 | 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 |
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)