Skip to content

Commit e90a889

Browse files
asger-semmleasger-semmle
committed
JS: Refactor DOM libs to use DataFlow more
1 parent 4908902 commit e90a889

5 files changed

Lines changed: 102 additions & 99 deletions

File tree

‎javascript/ql/src/semmle/javascript/DOM.qll‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,4 +253,86 @@ module DOM {
253253
reason = "must not contain any space characters"
254254
)
255255
}
256+
257+
/** Gets a call that queries the DOM for a collection of DOM nodes. */
258+
private DataFlow::SourceNode domElementCollection() {
259+
exists(string collectionName |
260+
collectionName = "getElementsByClassName" or
261+
collectionName = "getElementsByName" or
262+
collectionName = "getElementsByTagName" or
263+
collectionName = "getElementsByTagNameNS" or
264+
collectionName = "querySelectorAll"
265+
|
266+
(
267+
result = documentRef().getAMethodCall(collectionName) or
268+
result = DataFlow::globalVarRef(collectionName).getACall()
269+
)
270+
)
271+
}
272+
273+
/** Gets a call that creates a DOM node or queries the DOM for a DOM node. */
274+
private DataFlow::SourceNode domElementCreationOrQuery() {
275+
exists(string methodName |
276+
methodName = "createElement" or
277+
methodName = "createElementNS" or
278+
methodName = "createRange" or
279+
methodName = "getElementById" or
280+
methodName = "querySelector"
281+
|
282+
result = documentRef().getAMethodCall(methodName) or
283+
result = DataFlow::globalVarRef(methodName).getACall()
284+
)
285+
}
286+
287+
/** Gets a data flow node that refer directly to a value from the DOM. */
288+
DataFlow::SourceNode domValueSource() {
289+
result.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
290+
result = domValueRef().getAPropertyRead(_) or
291+
result = domElementCreationOrQuery() or
292+
result = domElementCollection()
293+
}
294+
295+
/** Gets a data flow node that may refer to a value from the DOM. */
296+
private DataFlow::SourceNode domValueRef(DataFlow::TypeTracker t) {
297+
t.start() and
298+
result = domValueSource()
299+
or
300+
exists(DataFlow::TypeTracker t2 | result = domValueRef(t2).track(t2, t))
301+
}
302+
303+
/** Gets a data flow node that may refer to a value from the DOM. */
304+
DataFlow::SourceNode domValueRef() { result = domValueRef(DataFlow::TypeTracker::end()) }
305+
306+
/** Gets a data flow node that directly refers to the DOM `location` object. */
307+
DataFlow::SourceNode locationSource() {
308+
result = domValueRef().getAPropertyRead("location")
309+
or
310+
result = DataFlow::globalVarRef("location")
311+
}
312+
313+
/** Gets a reference to the DOM `location` object. */
314+
private DataFlow::SourceNode locationRef(DataFlow::TypeTracker t) {
315+
t.start() and
316+
result = locationSource()
317+
or
318+
exists(DataFlow::TypeTracker t2 | result = locationRef(t2).track(t2, t))
319+
}
320+
321+
/** Gets a reference to the DOM `location` object. */
322+
DataFlow::SourceNode locationRef() { result = locationRef(DataFlow::TypeTracker::end()) }
323+
324+
/**
325+
* Gets a reference to the 'document' object.
326+
*/
327+
private DataFlow::SourceNode documentRef(DataFlow::TypeTracker t) {
328+
t.start() and
329+
result = DataFlow::globalVarRef("document")
330+
or
331+
exists(DataFlow::TypeTracker t2 | result = documentRef(t2).track(t2, t))
332+
}
333+
334+
/**
335+
* Gets a reference to the 'document' object.
336+
*/
337+
DataFlow::SourceNode documentRef() { result = documentRef(DataFlow::TypeTracker::end()) }
256338
}

‎javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,9 @@ module ClientSideUrlRedirect {
121121
)
122122
or
123123
// A call to `location.replace` or `location.assign`
124-
exists(MethodCallExpr locationCall, string name |
125-
isLocation(locationCall.getReceiver()) and
126-
name = locationCall.getMethodName() and
127-
astNode = locationCall.getArgument(0)
124+
exists(DataFlow::MethodCallNode locationCall, string name |
125+
locationCall = DOM::locationRef().getAMethodCall(name) and
126+
this = locationCall.getArgument(0)
128127
|
129128
name = "replace" or name = "assign"
130129
)
@@ -134,8 +133,7 @@ module ClientSideUrlRedirect {
134133
or
135134
// An assignment to `location.href`, `location.protocol` or `location.hostname`
136135
exists(DataFlow::PropWrite pw, string propName |
137-
isLocation(pw.getBase().asExpr()) and
138-
propName = pw.getPropertyName() and
136+
pw = DOM::locationRef().getAPropertyWrite(propName) and
139137
this = pw.getRhs()
140138
|
141139
propName = "href" or propName = "protocol" or propName = "hostname"

‎javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll‎

Lines changed: 14 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -18,113 +18,36 @@ class DOMGlobalVariable extends GlobalVariable {
1818
}
1919
}
2020

21-
private DataFlow::SourceNode domElementCollection() {
22-
exists(string collectionName |
23-
collectionName = "getElementsByClassName" or
24-
collectionName = "getElementsByName" or
25-
collectionName = "getElementsByTagName" or
26-
collectionName = "getElementsByTagNameNS" or
27-
collectionName = "querySelectorAll"
28-
|
29-
(
30-
result = document().getAMethodCall(collectionName) or
31-
result = DataFlow::globalVarRef(collectionName).getACall()
32-
)
33-
)
34-
}
35-
36-
private DataFlow::SourceNode domElementCreationOrQuery() {
37-
exists(string methodName |
38-
methodName = "createElement" or
39-
methodName = "createElementNS" or
40-
methodName = "createRange" or
41-
methodName = "getElementById" or
42-
methodName = "querySelector"
43-
|
44-
result = document().getAMethodCall(methodName) or
45-
result = DataFlow::globalVarRef(methodName).getACall()
46-
)
47-
}
48-
49-
private DataFlow::SourceNode domValueSource(DataFlow::TypeTracker t) {
50-
t.start() and
51-
(
52-
result.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
53-
result = domValueSource().getAPropertyRead(_) or
54-
result = domElementCreationOrQuery() or
55-
result = domElementCollection()
56-
)
57-
or
58-
exists(DataFlow::TypeTracker t2 |
59-
result = domValueSource(t2).track(t2, t)
60-
)
61-
}
62-
63-
private DataFlow::SourceNode domValueSource() {
64-
result = domValueSource(DataFlow::TypeTracker::end())
65-
}
66-
6721
/** Holds if `e` could hold a value that comes from the DOM. */
68-
predicate isDomValue(Expr e) { domValueSource().flowsToExpr(e) }
69-
70-
/** Gets a reference to the DOM `location` object. */
71-
private DataFlow::SourceNode locationRef(DataFlow::TypeTracker t) {
72-
t.start() and
73-
exists(Expr e | result.asExpr() = e |
74-
e = domValueSource().getAPropertyReference("location").asExpr() or
75-
e.accessesGlobal("location")
76-
)
77-
or
78-
exists(DataFlow::TypeTracker t2 |
79-
result = locationRef(t2).track(t2, t)
80-
)
81-
}
82-
83-
/** Gets a reference to the DOM `location` object. */
84-
private DataFlow::SourceNode locationRef() {
85-
result = locationRef(DataFlow::TypeTracker::end())
86-
}
22+
predicate isDomValue(Expr e) { DOM::domValueRef().flowsToExpr(e) }
8723

8824
/** Holds if `e` could refer to the `location` property of a DOM node. */
8925
predicate isLocation(Expr e) {
90-
locationRef().flowsToExpr(e)
91-
}
92-
93-
/**
94-
* Gets a reference to the 'document' object.
95-
*/
96-
private DataFlow::SourceNode document(DataFlow::TypeTracker t) {
97-
t.start() and
98-
result = DataFlow::globalVarRef("document")
26+
e = DOM::domValueRef().getAPropertyReference("location").asExpr()
9927
or
100-
exists(DataFlow::TypeTracker t2 |
101-
result = document(t2).track(t2, t)
102-
)
28+
e.accessesGlobal("location")
10329
}
10430

10531
/**
10632
* Gets a reference to the 'document' object.
10733
*/
108-
DataFlow::SourceNode document() { result = document(DataFlow::TypeTracker::end()) }
34+
DataFlow::SourceNode document() { result = DOM::documentRef() }
10935

11036
/** Holds if `e` could refer to the `document` object. */
111-
predicate isDocument(Expr e) { document().flowsToExpr(e) }
37+
predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
11238

11339
/** Holds if `e` could refer to the document URL. */
11440
predicate isDocumentURL(Expr e) {
115-
exists(Expr base, string propName | e.(PropAccess).accesses(base, propName) |
116-
isDocument(base) and
117-
(
118-
propName = "documentURI" or
119-
propName = "documentURIObject" or
120-
propName = "location" or
121-
propName = "referrer" or
122-
propName = "URL"
123-
)
124-
or
125-
isDomValue(base) and propName = "baseUri"
41+
exists(string propName | e = DOM::documentRef().getAPropertyRead(propName).asExpr() |
42+
propName = "documentURI" or
43+
propName = "documentURIObject" or
44+
propName = "location" or
45+
propName = "referrer" or
46+
propName = "URL"
12647
)
12748
or
49+
e = DOM::domValueRef().getAPropertyRead("baseUri").asExpr()
50+
or
12851
e.accessesGlobal("location")
12952
}
13053

@@ -134,7 +57,7 @@ predicate isDocumentURL(Expr e) {
13457
* `href`, `hash` and `search`.
13558
*/
13659
predicate isSafeLocationProperty(PropAccess pacc) {
137-
exists(Expr loc, string prop | isLocation(loc) and pacc.accesses(loc, prop) |
60+
exists(string prop | pacc = DOM::locationRef().getAPropertyRead(prop).asExpr() |
13861
prop != "href" and prop != "hash" and prop != "search"
13962
)
14063
}

‎javascript/ql/src/semmle/javascript/security/dataflow/PropertyInjectionShared.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ module PropertyInjection {
1414
node = DataFlow::globalObjectRef()
1515
or
1616
// document.write can be accessed
17-
isDocument(node.asExpr())
17+
node = DOM::documentRef()
1818
or
1919
// 'constructor' property leads to the Function constructor.
2020
node.analyze().getAValue() instanceof AbstractCallable

‎javascript/ql/src/semmle/javascript/security/dataflow/XpathInjection.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ module XpathInjection {
7575
class DomXpathSink extends Sink {
7676
DomXpathSink() {
7777
exists(string m | m = "evaluate" or m = "createExpression" |
78-
this = document().getAMethodCall(m).getArgument(0)
78+
this = DOM::documentRef().getAMethodCall(m).getArgument(0)
7979
)
8080
}
8181
}

0 commit comments

Comments
 (0)