Skip to content

Commit 18cfe72

Browse files
committed
JS: Add model of d3
1 parent ad665b7 commit 18cfe72

File tree

6 files changed

+143
-2
lines changed

6 files changed

+143
-2
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
lgtm,codescanning
2+
* Support for `d3` has improved. The XSS queries now recognize HTML injection sinks
3+
from the `d3` API.
4+
Affected packages are
5+
[d3](https://npmjs.com/package/d3),
6+
[d3-selection](https://npmjs.com/package/d3-selection).

javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import semmle.javascript.frameworks.ClosureLibrary
8080
import semmle.javascript.frameworks.CookieLibraries
8181
import semmle.javascript.frameworks.Credentials
8282
import semmle.javascript.frameworks.CryptoLibraries
83+
import semmle.javascript.frameworks.D3
8384
import semmle.javascript.frameworks.DateFunctions
8485
import semmle.javascript.frameworks.DigitalOcean
8586
import semmle.javascript.frameworks.Electron
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/** Provides classes and predicates modelling aspects of the `d3` library. */
2+
private import javascript
3+
private import semmle.javascript.security.dataflow.Xss
4+
5+
/** Provides classes and predicates modelling aspects of the `d3` library. */
6+
module D3 {
7+
/** Gets an API node referring to the `d3` module. */
8+
API::Node d3() {
9+
result = API::moduleImport("d3")
10+
or
11+
result = API::moduleImport("d3-node").getInstance().getMember("d3")
12+
}
13+
14+
/**
15+
* Gets an API node referring to the given module, or to `d3`.
16+
*
17+
* Useful for accessing modules that are re-exported by `d3`.
18+
*/
19+
bindingset[name]
20+
API::Node d3Module(string name) {
21+
result = d3() // all d3 modules are re-exported as part of 'd3'
22+
or
23+
result = API::moduleImport(name)
24+
}
25+
26+
/** Gets an API node referring to a `d3` selection object, such as `d3.selection()`. */
27+
API::Node d3Selection() {
28+
result = d3Module("d3-selection").getMember(["selection", "select", "selectAll"]).getReturn()
29+
or
30+
exists(API::CallNode call, string name |
31+
call = d3Selection().getMember(name).getACall() and
32+
result = call.getReturn()
33+
|
34+
name = ["select", "selectAll", "filter", "merge", "selectChild", "selectChildren", "selection", "insert", "remove", "clone", "sort", "order", "raise", "lower", "append", "data", "join", "enter", "exit", "call"]
35+
or
36+
name = ["text", "html", "datum"] and
37+
call.getNumArgument() > 0 // exclude 0-argument version, which returns the current value
38+
or
39+
name = ["attr", "classed", "style", "property", "on"] and
40+
call.getNumArgument() > 1 // exclude 1-argument version, which returns the current value
41+
)
42+
or
43+
result = d3Selection().getMember("call").getParameter(0).getParameter(0)
44+
}
45+
46+
private class D3XssSink extends DomBasedXss::Sink {
47+
D3XssSink() {
48+
exists(API::Node htmlArg |
49+
htmlArg = d3Selection().getMember("html").getParameter(0) and
50+
this = [htmlArg, htmlArg.getReturn()].getARhs()
51+
)
52+
}
53+
}
54+
55+
private class D3DomValueSource extends DOM::DomValueSource::Range {
56+
D3DomValueSource() {
57+
this = d3Selection().getMember("each").getReceiver().getAnImmediateUse()
58+
or
59+
this = d3Selection().getMember("node").getReturn().getAnImmediateUse()
60+
or
61+
this = d3Selection().getMember("nodes").getReturn().getUnknownMember().getAnImmediateUse()
62+
}
63+
}
64+
65+
private class D3AttributeDefinition extends DOM::AttributeDefinition {
66+
DataFlow::CallNode call;
67+
68+
D3AttributeDefinition() {
69+
call = d3Selection().getMember("attr").getACall() and
70+
call.getNumArgument() > 1 and
71+
this = call.asExpr()
72+
}
73+
74+
override string getName() {
75+
result = call.getArgument(0).getStringValue()
76+
}
77+
78+
override DataFlow::Node getValueNode() {
79+
result = call.getArgument(1)
80+
}
81+
}
82+
}

javascript/ql/src/semmle/javascript/frameworks/jQuery.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,8 +479,8 @@ module JQuery {
479479
// either a reference to a global variable `$` or `jQuery`
480480
this = DataFlow::globalVarRef(any(string jq | jq = "$" or jq = "jQuery"))
481481
or
482-
// or imported from a module named `jquery`
483-
this = DataFlow::moduleImport("jquery")
482+
// or imported from a module named `jquery` or `zepto`
483+
this = DataFlow::moduleImport(["jquery", "zepto"])
484484
or
485485
this.hasUnderlyingType("JQueryStatic")
486486
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,16 @@ nodes
9292
| classnames.js:15:47:15:63 | clsx(window.name) |
9393
| classnames.js:15:52:15:62 | window.name |
9494
| classnames.js:15:52:15:62 | window.name |
95+
| d3.js:4:12:4:22 | window.name |
96+
| d3.js:4:12:4:22 | window.name |
97+
| d3.js:11:15:11:24 | getTaint() |
98+
| d3.js:11:15:11:24 | getTaint() |
99+
| d3.js:12:20:12:29 | getTaint() |
100+
| d3.js:12:20:12:29 | getTaint() |
101+
| d3.js:14:20:14:29 | getTaint() |
102+
| d3.js:14:20:14:29 | getTaint() |
103+
| d3.js:21:15:21:24 | getTaint() |
104+
| d3.js:21:15:21:24 | getTaint() |
95105
| dates.js:9:9:9:69 | taint |
96106
| dates.js:9:17:9:69 | decodeU ... ing(1)) |
97107
| dates.js:9:36:9:50 | window.location |
@@ -772,6 +782,22 @@ edges
772782
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
773783
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
774784
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
785+
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
786+
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
787+
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
788+
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
789+
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
790+
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
791+
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
792+
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
793+
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
794+
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
795+
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
796+
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
797+
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
798+
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
799+
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
800+
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
775801
| dates.js:9:9:9:69 | taint | dates.js:11:63:11:67 | taint |
776802
| dates.js:9:9:9:69 | taint | dates.js:12:66:12:70 | taint |
777803
| dates.js:9:9:9:69 | taint | dates.js:13:59:13:63 | taint |
@@ -1338,6 +1364,10 @@ edges
13381364
| 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 |
13391365
| 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 |
13401366
| 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 |
1367+
| d3.js:11:15:11:24 | getTaint() | d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() | Cross-site scripting vulnerability due to $@. | d3.js:4:12:4:22 | window.name | user-provided value |
1368+
| d3.js:12:20:12:29 | getTaint() | d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() | Cross-site scripting vulnerability due to $@. | d3.js:4:12:4:22 | window.name | user-provided value |
1369+
| d3.js:14:20:14:29 | getTaint() | d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() | Cross-site scripting vulnerability due to $@. | d3.js:4:12:4:22 | window.name | user-provided value |
1370+
| d3.js:21:15:21:24 | getTaint() | d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() | Cross-site scripting vulnerability due to $@. | d3.js:4:12:4:22 | window.name | user-provided value |
13411371
| dates.js:11:31:11:70 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.location | dates.js:11:31:11:70 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.location | user-provided value |
13421372
| dates.js:12:31:12:73 | `Time i ... aint)}` | dates.js:9:36:9:50 | window.location | dates.js:12:31:12:73 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.location | user-provided value |
13431373
| dates.js:13:31:13:72 | `Time i ... time)}` | dates.js:9:36:9:50 | window.location | dates.js:13:31:13:72 | `Time i ... time)}` | Cross-site scripting vulnerability due to $@. | dates.js:9:36:9:50 | window.location | user-provided value |
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const d3 = require('d3');
2+
3+
function getTaint() {
4+
return window.name;
5+
}
6+
7+
function doSomething() {
8+
d3.select('#main')
9+
.attr('width', 100)
10+
.style('color', 'red')
11+
.html(getTaint()) // NOT OK
12+
.html(d => getTaint()) // NOT OK
13+
.call(otherFunction)
14+
.html(d => getTaint()); // NOT OK
15+
}
16+
17+
18+
function otherFunction(selection) {
19+
selection
20+
.attr('foo', 'bar')
21+
.html(getTaint()); // NOT OK
22+
}

0 commit comments

Comments
 (0)