Skip to content

Commit 5c2f116

Browse files
authored
Merge pull request #3679 from asger-semmle/js/dom-value-ref-restriction
Approved by erik-krogh, esbena
2 parents 243e3ad + 4bb2e8b commit 5c2f116

5 files changed

Lines changed: 58 additions & 2 deletions

File tree

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,27 @@ module DOM {
291291
*/
292292
abstract class Range extends DataFlow::Node { }
293293

294+
private string getADomPropertyName() {
295+
exists(ExternalInstanceMemberDecl decl |
296+
result = decl.getName() and
297+
isDomRootType(decl.getDeclaringType().getASupertype*())
298+
)
299+
}
300+
294301
private class DefaultRange extends Range {
295302
DefaultRange() {
296303
this.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable
297304
or
298-
this = domValueRef().getAPropertyRead()
305+
exists(DataFlow::PropRead read |
306+
this = read and
307+
read = domValueRef().getAPropertyRead()
308+
|
309+
not read.mayHavePropertyName(_)
310+
or
311+
read.mayHavePropertyName(getADomPropertyName())
312+
or
313+
read.mayHavePropertyName(any(string s | exists(s.toInt())))
314+
)
299315
or
300316
this = domElementCreationOrQuery()
301317
or

javascript/ql/test/library-tests/DOM/Customizations.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@ test_locationRef
44
| customization.js:3:3:3:14 | doc.location |
55
test_domValueRef
66
| customization.js:4:3:4:28 | doc.get ... 'test') |
7+
| tst.js:49:3:49:8 | window |
8+
| tst.js:50:3:50:8 | window |
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/** @externs */
2+
3+
/**
4+
* @constructor
5+
* @name EventTarget
6+
*/
7+
function EventTarget() {}
8+
9+
/** @type {EventTarget} */
10+
var window;

javascript/ql/test/library-tests/DOM/tst.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,13 @@
3939
factory2();
4040

4141
})();
42+
43+
(function pollute() {
44+
class C {
45+
foo() {
46+
this.x; // Should not be a domValueRef
47+
}
48+
}
49+
window.myApp = new C();
50+
window.myApp.foo();
51+
})();

javascript/ql/test/query-tests/Security/CWE-079/externs.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,24 @@
2525
function EventTarget() {}
2626

2727
/**
28-
* @type {!EventTarget}
28+
* Stub for the DOM hierarchy.
29+
*
30+
* @constructor
31+
* @extends {EventTarget}
32+
*/
33+
function DomObjectStub() {}
34+
35+
/**
36+
* @type {!DomObjectStub}
37+
*/
38+
DomObjectStub.prototype.body;
39+
40+
/**
41+
* @type {!DomObjectStub}
42+
*/
43+
DomObjectStub.prototype.value;
44+
45+
/**
46+
* @type {!DomObjectStub}
2947
*/
3048
var document;

0 commit comments

Comments
 (0)