Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions javascript/change-notes/2021-03-10-d3.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
lgtm,codescanning
* Support for `d3` has improved. The XSS queries now recognize HTML injection sinks
from the `d3` API.
Affected packages are
[d3](https://npmjs.com/package/d3),
[d3-selection](https://npmjs.com/package/d3-selection).
1 change: 1 addition & 0 deletions javascript/ql/src/javascript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import semmle.javascript.frameworks.ClosureLibrary
import semmle.javascript.frameworks.CookieLibraries
import semmle.javascript.frameworks.Credentials
import semmle.javascript.frameworks.CryptoLibraries
import semmle.javascript.frameworks.D3
import semmle.javascript.frameworks.DateFunctions
import semmle.javascript.frameworks.DigitalOcean
import semmle.javascript.frameworks.Electron
Expand Down
102 changes: 102 additions & 0 deletions javascript/ql/src/semmle/javascript/frameworks/D3.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/** Provides classes and predicates modelling aspects of the `d3` library. */

private import javascript
private import semmle.javascript.security.dataflow.Xss

/** Provides classes and predicates modelling aspects of the `d3` library. */
module D3 {
/** The global variable `d3` as an entry point for API graphs. */
private class D3GlobalEntry extends API::EntryPoint {
D3GlobalEntry() { this = "D3GlobalEntry" }

override DataFlow::SourceNode getAUse() { result = DataFlow::globalVarRef("d3") }

override DataFlow::Node getARhs() { none() }
}

/** Gets an API node referring to the `d3` module. */
API::Node d3() {
result = API::moduleImport("d3")
or
// recognize copies of d3 in a scope
result = API::moduleImport(any(string s | s.regexpMatch("@.*/d3(-\\w+)?")))
or
result = API::moduleImport("d3-node").getInstance().getMember("d3")
or
result = API::root().getASuccessor(any(D3GlobalEntry i))
}

/**
* Gets an API node referring to the given module, or to `d3`.
*
* Useful for accessing modules that are re-exported by `d3`.
*/
bindingset[name]
API::Node d3Module(string name) {
result = d3() // all d3 modules are re-exported as part of 'd3'
or
result = API::moduleImport(name)
}

/** Gets an API node referring to a `d3` selection object, such as `d3.selection()`. */
API::Node d3Selection() {
result = d3Module("d3-selection").getMember(["selection", "select", "selectAll"]).getReturn()
or
exists(API::CallNode call, string name |
call = d3Selection().getMember(name).getACall() and
result = call.getReturn()
|
name =
[
"select", "selectAll", "filter", "merge", "selectChild", "selectChildren", "selection",
"insert", "remove", "clone", "sort", "order", "raise", "lower", "append", "data", "join",
"enter", "exit", "call"
]
or
name = ["text", "html", "datum"] and
call.getNumArgument() > 0 // exclude 0-argument version, which returns the current value
or
name = ["attr", "classed", "style", "property", "on"] and
call.getNumArgument() > 1 // exclude 1-argument version, which returns the current value
or
// Setting multiple things at once
name = ["attr", "classed", "style", "property", "on"] and
call.getArgument(0).getALocalSource() instanceof DataFlow::ObjectLiteralNode
)
or
result = d3Selection().getMember("call").getParameter(0).getParameter(0)
}

private class D3XssSink extends DomBasedXss::Sink {
D3XssSink() {
exists(API::Node htmlArg |
htmlArg = d3Selection().getMember("html").getParameter(0) and
this = [htmlArg, htmlArg.getReturn()].getARhs()
)
}
}

private class D3DomValueSource extends DOM::DomValueSource::Range {
D3DomValueSource() {
this = d3Selection().getMember("each").getReceiver().getAnImmediateUse()
or
this = d3Selection().getMember("node").getReturn().getAnImmediateUse()
or
this = d3Selection().getMember("nodes").getReturn().getUnknownMember().getAnImmediateUse()
}
}

private class D3AttributeDefinition extends DOM::AttributeDefinition {
DataFlow::CallNode call;

D3AttributeDefinition() {
call = d3Selection().getMember("attr").getACall() and
call.getNumArgument() > 1 and
this = call.asExpr()
}

override string getName() { result = call.getArgument(0).getStringValue() }

override DataFlow::Node getValueNode() { result = call.getArgument(1) }
}
}
4 changes: 2 additions & 2 deletions javascript/ql/src/semmle/javascript/frameworks/jQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,8 @@ module JQuery {
// either a reference to a global variable `$` or `jQuery`
this = DataFlow::globalVarRef(any(string jq | jq = "$" or jq = "jQuery"))
or
// or imported from a module named `jquery`
this = DataFlow::moduleImport("jquery")
// or imported from a module named `jquery` or `zepto`
this = DataFlow::moduleImport(["jquery", "zepto"])
or
this.hasUnderlyingType("JQueryStatic")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ module XssThroughDom {
result = ["name", "value", "title", "alt"]
}

/**
* Gets a DOM property name that could store user-controlled data.
*/
string unsafeDomPropertyName() { result = ["innerText", "textContent", "value", "name"] }

/**
* A source for text from the DOM from a JQuery method call.
*/
Expand All @@ -35,10 +40,16 @@ module XssThroughDom {
(
this.getMethodName() = ["text", "val"] and this.getNumArgument() = 0
or
this.getMethodName() = "attr" and
this.getNumArgument() = 1 and
forex(InferredType t | t = this.getArgument(0).analyze().getAType() | t = TTString()) and
this.getArgument(0).mayHaveStringValue(unsafeAttributeName())
exists(string methodName, string value |
this.getMethodName() = methodName and
this.getNumArgument() = 1 and
forex(InferredType t | t = this.getArgument(0).analyze().getAType() | t = TTString()) and
this.getArgument(0).mayHaveStringValue(value)
|
methodName = "attr" and value = unsafeAttributeName()
or
methodName = "prop" and value = unsafeDomPropertyName()
)
) and
// looks like a $("<p>" + ... ) source, which is benign for this query.
not exists(DataFlow::Node prefix |
Expand All @@ -51,14 +62,37 @@ module XssThroughDom {
}
}

/**
* A source for text from the DOM from a `d3` method call.
*/
class D3TextSource extends Source {
D3TextSource() {
exists(DataFlow::MethodCallNode call, string methodName |
this = call and
call = D3::d3Selection().getMember(methodName).getACall()
|
methodName = "attr" and
call.getNumArgument() = 1 and
call.getArgument(0).mayHaveStringValue(unsafeAttributeName())
or
methodName = "property" and
call.getNumArgument() = 1 and
call.getArgument(0).mayHaveStringValue(unsafeDomPropertyName())
or
methodName = "text" and
call.getNumArgument() = 0
)
}
}

/**
* A source for text from the DOM from a DOM property read or call to `getAttribute()`.
*/
class DOMTextSource extends Source {
DOMTextSource() {
exists(DataFlow::PropRead read | read = this |
read.getBase().getALocalSource() = DOM::domValueRef() and
read.mayHavePropertyName(["innerText", "textContent", "value", "name"])
read.mayHavePropertyName(unsafeDomPropertyName())
)
or
exists(DataFlow::MethodCallNode mcn | mcn = this |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ nodes
| classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name |
| classnames.js:15:52:15:62 | window.name |
| d3.js:4:12:4:22 | window.name |
| d3.js:4:12:4:22 | window.name |
| d3.js:11:15:11:24 | getTaint() |
| d3.js:11:15:11:24 | getTaint() |
| d3.js:12:20:12:29 | getTaint() |
| d3.js:12:20:12:29 | getTaint() |
| d3.js:14:20:14:29 | getTaint() |
| d3.js:14:20:14:29 | getTaint() |
| d3.js:21:15:21:24 | getTaint() |
| d3.js:21:15:21:24 | getTaint() |
| dates.js:9:9:9:69 | taint |
| dates.js:9:17:9:69 | decodeU ... ing(1)) |
| dates.js:9:36:9:50 | window.location |
Expand Down Expand Up @@ -772,6 +782,22 @@ edges
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| dates.js:9:9:9:69 | taint | dates.js:11:63:11:67 | taint |
| dates.js:9:9:9:69 | taint | dates.js:12:66:12:70 | taint |
| dates.js:9:9:9:69 | taint | dates.js:13:59:13:63 | taint |
Expand Down Expand Up @@ -1338,6 +1364,10 @@ edges
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ nodes
| classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name |
| classnames.js:15:52:15:62 | window.name |
| d3.js:4:12:4:22 | window.name |
| d3.js:4:12:4:22 | window.name |
| d3.js:11:15:11:24 | getTaint() |
| d3.js:11:15:11:24 | getTaint() |
| d3.js:12:20:12:29 | getTaint() |
| d3.js:12:20:12:29 | getTaint() |
| d3.js:14:20:14:29 | getTaint() |
| d3.js:14:20:14:29 | getTaint() |
| d3.js:21:15:21:24 | getTaint() |
| d3.js:21:15:21:24 | getTaint() |
| dates.js:9:9:9:69 | taint |
| dates.js:9:17:9:69 | decodeU ... ing(1)) |
| dates.js:9:36:9:50 | window.location |
Expand Down Expand Up @@ -783,6 +793,22 @@ edges
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:11:15:11:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:12:20:12:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:14:20:14:29 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| d3.js:4:12:4:22 | window.name | d3.js:21:15:21:24 | getTaint() |
| dates.js:9:9:9:69 | taint | dates.js:11:63:11:67 | taint |
| dates.js:9:9:9:69 | taint | dates.js:12:66:12:70 | taint |
| dates.js:9:9:9:69 | taint | dates.js:13:59:13:63 | taint |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const d3 = require('d3');

function getTaint() {
return window.name;
}

function doSomething() {
d3.select('#main')
.attr('width', 100)
.style('color', 'red')
.html(getTaint()) // NOT OK
.html(d => getTaint()) // NOT OK
.call(otherFunction)
.html(d => getTaint()); // NOT OK
}


function otherFunction(selection) {
selection
.attr('foo', 'bar')
.html(getTaint()); // NOT OK
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ nodes
| xss-through-dom.js:79:4:79:34 | documen ... t.value |
| xss-through-dom.js:79:4:79:34 | documen ... t.value |
| xss-through-dom.js:79:4:79:34 | documen ... t.value |
| xss-through-dom.js:81:17:81:43 | $('#foo ... rText') |
| xss-through-dom.js:81:17:81:43 | $('#foo ... rText') |
| xss-through-dom.js:81:17:81:43 | $('#foo ... rText') |
edges
| forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values |
| forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values |
Expand Down Expand Up @@ -157,6 +160,7 @@ edges
| xss-through-dom.js:73:20:73:41 | $("inpu ... 0).name | xss-through-dom.js:73:9:73:41 | selector |
| xss-through-dom.js:73:20:73:41 | $("inpu ... 0).name | xss-through-dom.js:73:9:73:41 | selector |
| xss-through-dom.js:79:4:79:34 | documen ... t.value | xss-through-dom.js:79:4:79:34 | documen ... t.value |
| xss-through-dom.js:81:17:81:43 | $('#foo ... rText') | xss-through-dom.js:81:17:81:43 | $('#foo ... rText') |
#select
| forms.js:9:31:9:40 | values.foo | forms.js:8:23:8:28 | values | forms.js:9:31:9:40 | values.foo | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:8:23:8:28 | values | DOM text |
| forms.js:12:31:12:40 | values.bar | forms.js:11:24:11:29 | values | forms.js:12:31:12:40 | values.bar | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:11:24:11:29 | values | DOM text |
Expand Down Expand Up @@ -185,3 +189,4 @@ edges
| xss-through-dom.js:71:11:71:32 | $("inpu ... 0).name | xss-through-dom.js:71:11:71:32 | $("inpu ... 0).name | xss-through-dom.js:71:11:71:32 | $("inpu ... 0).name | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:71:11:71:32 | $("inpu ... 0).name | DOM text |
| xss-through-dom.js:77:4:77:11 | selector | xss-through-dom.js:73:20:73:41 | $("inpu ... 0).name | xss-through-dom.js:77:4:77:11 | selector | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:73:20:73:41 | $("inpu ... 0).name | DOM text |
| xss-through-dom.js:79:4:79:34 | documen ... t.value | xss-through-dom.js:79:4:79:34 | documen ... t.value | xss-through-dom.js:79:4:79:34 | documen ... t.value | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:79:4:79:34 | documen ... t.value | DOM text |
| xss-through-dom.js:81:17:81:43 | $('#foo ... rText') | xss-through-dom.js:81:17:81:43 | $('#foo ... rText') | xss-through-dom.js:81:17:81:43 | $('#foo ... rText') | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:81:17:81:43 | $('#foo ... rText') | DOM text |
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,6 @@
$(selector); // NOT OK

$(document.my_form.my_input.value); // NOT OK

$("#id").html( $('#foo').prop('innerText') ); // NOT OK
})();