Skip to content

Commit fb181c2

Browse files
committed
JS: Use type info and type tracking in jQuery
1 parent abdf7ce commit fb181c2

File tree

9 files changed

+210
-41
lines changed

9 files changed

+210
-41
lines changed

javascript/ql/src/semmle/javascript/Extend.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ private class ExtendCallWithFlag extends ExtendCall {
4747
name = "node.extend"
4848
)
4949
or
50-
this = jquery().getAPropertyRead("extend").getACall()
50+
// Match $.extend using the source of `$` only, as ExtendCall should not
51+
// depend on type tracking.
52+
this = JQuery::dollarSource().getAMemberCall("extend")
5153
}
5254

5355
/**

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

Lines changed: 157 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,50 +7,51 @@ import javascript
77
/**
88
* Gets a data flow node that may refer to the jQuery `$` function.
99
*/
10-
DataFlow::SourceNode jquery() {
11-
// either a reference to a global variable `$` or `jQuery`
12-
result = DataFlow::globalVarRef(any(string jq | jq = "$" or jq = "jQuery"))
13-
or
14-
// or imported from a module named `jquery`
15-
result = DataFlow::moduleImport("jquery")
16-
}
10+
predicate jquery = JQuery::dollar/0;
1711

1812
/**
13+
* DEPRECATED. In most cases, `JQuery::Object` should be used instead.
14+
* Alternatively, if used as a base class, and the intent is to extend the model of
15+
* jQuery objects with more nodes, extend `JQuery::ObjectSource::Range` instead.
16+
*
1917
* An expression that may refer to a jQuery object.
2018
*
2119
* Note that this class is an over-approximation: `nd instanceof JQueryObject`
2220
* may hold for nodes `nd` that cannot, in fact, refer to a jQuery object.
2321
*/
24-
abstract class JQueryObject extends Expr { }
22+
deprecated class JQueryObject = JQueryObjectInternal;
23+
24+
/**
25+
* An internal version of `JQueryObject` that may be used to retain
26+
* backwards compatibility without triggering a deprecation warning.
27+
*/
28+
abstract private class JQueryObjectInternal extends Expr { }
2529

2630
/**
2731
* A jQuery object created from a jQuery method.
32+
*
33+
* This class is defined using the legacy API in order to retain the
34+
* behavior of `JQueryObject`.
2835
*/
29-
private class OrdinaryJQueryObject extends JQueryObject {
36+
private class OrdinaryJQueryObject extends JQueryObjectInternal {
3037
OrdinaryJQueryObject() {
31-
exists(JQueryMethodCall jq |
32-
this.flow().getALocalSource().asExpr() = jq and
38+
exists(JQuery::MethodCall jq |
39+
this.flow().getALocalSource() = jq and
3340
// `jQuery.val()` does _not_ return a jQuery object
3441
jq.getMethodName() != "val"
3542
)
3643
}
3744
}
3845

3946
/**
47+
* DEPRECATED. Use `JQuery::MethodCall` instead.
48+
*
4049
* A (possibly chained) call to a jQuery method.
4150
*/
42-
class JQueryMethodCall extends CallExpr {
51+
deprecated class JQueryMethodCall extends CallExpr {
4352
string name;
4453

45-
JQueryMethodCall() {
46-
this = jquery().getACall().asExpr() and name = "$"
47-
or
48-
// initial call
49-
this = jquery().getAMemberCall(name).asExpr()
50-
or
51-
// chained call
52-
this.(MethodCallExpr).calls(any(JQueryObject jq), name)
53-
}
54+
JQueryMethodCall() { name = this.flow().(JQuery::MethodCall).getMethodName() }
5455

5556
/**
5657
* Gets the name of the jQuery method this call invokes.
@@ -65,13 +66,7 @@ class JQueryMethodCall extends CallExpr {
6566
* `interpretsArgumentAsSelector` below overlap.
6667
*/
6768
predicate interpretsArgumentAsHtml(Expr e) {
68-
// some methods interpret all their arguments as (potential) HTML
69-
JQuery::isMethodArgumentInterpretedAsHtml(name) and
70-
e = getAnArgument()
71-
or
72-
// for `$, it's only the first one
73-
name = "$" and
74-
e = getArgument(0)
69+
this.flow().(JQuery::MethodCall).interpretsArgumentAsHtml(e.flow())
7570
}
7671

7772
/**
@@ -82,21 +77,15 @@ class JQueryMethodCall extends CallExpr {
8277
* `interpretsArgumentAsHtml` above overlap.
8378
*/
8479
predicate interpretsArgumentAsSelector(Expr e) {
85-
// some methods interpret all their arguments as (potential) selectors
86-
JQuery::isMethodArgumentInterpretedAsSelector(name) and
87-
e = getAnArgument()
88-
or
89-
// for `$, it's only the first one
90-
name = "$" and
91-
e = getArgument(0)
80+
this.flow().(JQuery::MethodCall).interpretsArgumentAsSelector(e.flow())
9281
}
9382
}
9483

9584
/**
9685
* A call to `jQuery.parseXML`.
9786
*/
9887
private class JQueryParseXmlCall extends XML::ParserInvocation {
99-
JQueryParseXmlCall() { this.(JQueryMethodCall).getMethodName() = "parseXML" }
88+
JQueryParseXmlCall() { flow().(JQuery::MethodCall).getMethodName() = "parseXML" }
10089

10190
override Expr getSourceArgument() { result = getArgument(0) }
10291

@@ -249,13 +238,12 @@ private class JQueryAttr3Call extends JQueryAttributeDefinition, @callexpr {
249238
* For example, the call `$("<script/>").attr("src", mySource)` returns
250239
* the DOM element constructed by `$("<script/>")`.
251240
*/
252-
private class JQueryChainedElement extends DOM::Element {
241+
private class JQueryChainedElement extends DOM::Element, InvokeExpr {
253242
DOM::Element inner;
254243

255244
JQueryChainedElement() {
256-
exists(JQueryMethodCall jqmc | this = jqmc |
257-
jqmc.(MethodCallExpr).getReceiver() = inner and
258-
this instanceof JQueryObject and
245+
exists(JQuery::MethodCall call | this = call.asExpr() |
246+
call.getReceiver().asExpr() = inner and
259247
defn = inner.getDefinition()
260248
)
261249
}
@@ -318,4 +306,133 @@ module JQuery {
318306
name = "wrapAll" or
319307
name = "wrapInner"
320308
}
309+
310+
module DollarSource {
311+
/** A data flow node that may refer to the jQuery `$` function. */
312+
abstract class Range extends DataFlow::Node { }
313+
314+
private class DefaultRange extends Range {
315+
DefaultRange() {
316+
// either a reference to a global variable `$` or `jQuery`
317+
this = DataFlow::globalVarRef(any(string jq | jq = "$" or jq = "jQuery"))
318+
or
319+
// or imported from a module named `jquery`
320+
this = DataFlow::moduleImport("jquery")
321+
or
322+
this.hasUnderlyingType("JQueryStatic")
323+
}
324+
}
325+
}
326+
327+
/**
328+
* Gets a data flow node that may refer to the jQuery `$` function.
329+
*/
330+
DataFlow::SourceNode dollarSource() { result instanceof DollarSource::Range }
331+
332+
/** Gets a data flow node referring to the jQuery `$` function. */
333+
private DataFlow::SourceNode dollar(DataFlow::TypeTracker t) {
334+
t.start() and
335+
result = dollarSource()
336+
or
337+
exists(DataFlow::TypeTracker t2 | result = dollar(t2).track(t2, t))
338+
}
339+
340+
/** Gets a data flow node referring to the jQuery `$` function. */
341+
DataFlow::SourceNode dollar() { result = dollar(DataFlow::TypeTracker::end()) }
342+
343+
/** Gets an invocation of the jQuery `$` function. */
344+
DataFlow::CallNode dollarCall() { result = dollar().getACall() }
345+
346+
/** A call to the jQuery `$` function. */
347+
class DollarCall extends DataFlow::CallNode {
348+
DollarCall() { this = dollarCall() }
349+
}
350+
351+
module ObjectSource {
352+
/**
353+
* A data flow node that should be considered a source of jQuery objects.
354+
*/
355+
abstract class Range extends DataFlow::Node { }
356+
357+
private class DefaultRange extends Range {
358+
DefaultRange() {
359+
this.asExpr() instanceof JQueryObjectInternal
360+
or
361+
hasUnderlyingType("JQuery")
362+
or
363+
hasUnderlyingType("jQuery")
364+
}
365+
}
366+
}
367+
368+
/** Gets a source of jQuery objects. */
369+
private DataFlow::SourceNode objectSource() { result instanceof ObjectSource::Range }
370+
371+
/** Gets a data flow node referring to a jQuery object. */
372+
private DataFlow::SourceNode objectRef(DataFlow::TypeTracker t) {
373+
t.start() and
374+
result = objectSource()
375+
or
376+
exists(DataFlow::TypeTracker t2 | result = objectRef(t2).track(t2, t))
377+
}
378+
379+
/** Gets a data flow node referring to a jQuery object. */
380+
DataFlow::SourceNode objectRef() { result = objectRef(DataFlow::TypeTracker::end()) }
381+
382+
/** A data flow node that refers to a jQuery object. */
383+
class Object extends DataFlow::SourceNode {
384+
Object() { this = objectRef() }
385+
}
386+
387+
/** A call to a method on a jQuery object or the jQuery dollar function. */
388+
class MethodCall extends DataFlow::CallNode {
389+
string name;
390+
391+
MethodCall() {
392+
this = dollarCall() and name = "$"
393+
or
394+
this = dollar().getAMemberCall(name)
395+
or
396+
this = objectRef().getAMethodCall(name)
397+
}
398+
399+
/**
400+
* Gets the name of the jQuery method this call invokes.
401+
*/
402+
string getMethodName() { result = name }
403+
404+
/**
405+
* Holds if `node` is an argument that this method may interpret as HTML.
406+
*
407+
* Note that some jQuery methods decide whether to interpret an argument
408+
* as HTML based on its syntactic shape, so this predicate and
409+
* `interpretsArgumentAsSelector` below overlap.
410+
*/
411+
predicate interpretsArgumentAsHtml(DataFlow::Node node) {
412+
// some methods interpret all their arguments as (potential) HTML
413+
JQuery::isMethodArgumentInterpretedAsHtml(name) and
414+
node = getAnArgument()
415+
or
416+
// for `$, it's only the first one
417+
name = "$" and
418+
node = getArgument(0)
419+
}
420+
421+
/**
422+
* Holds if `node` is an argument that this method may interpret as a selector.
423+
*
424+
* Note that some jQuery methods decide whether to interpret an argument
425+
* as a selector based on its syntactic shape, so this predicate and
426+
* `interpretsArgumentAsHtml` above overlap.
427+
*/
428+
predicate interpretsArgumentAsSelector(DataFlow::Node node) {
429+
// some methods interpret all their arguments as (potential) selectors
430+
JQuery::isMethodArgumentInterpretedAsSelector(name) and
431+
node = getAnArgument()
432+
or
433+
// for `$, it's only the first one
434+
name = "$" and
435+
node = getArgument(0)
436+
}
437+
}
321438
}

javascript/ql/test/library-tests/frameworks/jQuery/JQueryMethodCall.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
| tracking.js:7:5:7:24 | this.elm.html('foo') | html |
2+
| tracking.js:14:5:14:17 | e.html('foo') | html |
3+
| tracking.js:18:7:18:15 | $('#foo') | $ |
4+
| ts_global_types.ts:3:5:3:17 | e.html('foo') | html |
5+
| ts_global_types.ts:7:1:7:9 | $('#foo') | $ |
6+
| ts_import.ts:5:5:5:17 | e.html('foo') | html |
17
| tst2.js:2:1:2:7 | jq("a") | $ |
28
| tst.js:1:1:1:3 | $() | $ |
39
| tst.js:2:1:2:8 | jQuery() | $ |
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
| tracking.js:7:5:7:24 | this.elm.html('foo') | tracking.js:7:19:7:23 | 'foo' |
2+
| tracking.js:14:5:14:17 | e.html('foo') | tracking.js:14:12:14:16 | 'foo' |
3+
| tracking.js:18:7:18:15 | $('#foo') | tracking.js:18:9:18:14 | '#foo' |
4+
| ts_global_types.ts:3:5:3:17 | e.html('foo') | ts_global_types.ts:3:12:3:16 | 'foo' |
5+
| ts_global_types.ts:7:1:7:9 | $('#foo') | ts_global_types.ts:7:3:7:8 | '#foo' |
6+
| ts_import.ts:5:5:5:17 | e.html('foo') | ts_import.ts:5:12:5:16 | 'foo' |
17
| tst2.js:2:1:2:7 | jq("a") | tst2.js:2:4:2:6 | "a" |
28
| tst.js:3:1:3:9 | $("<a/>") | tst.js:3:3:3:8 | "<a/>" |
39
| tst.js:7:1:7:14 | $("<br>", doc) | tst.js:7:3:7:8 | "<br>" |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
interface JQuery {}
2+
3+
declare module 'jquery' {
4+
export = JQuery;
5+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class C {
2+
constructor(elm) {
3+
this.elm = elm;
4+
}
5+
6+
doSomething() {
7+
this.elm.html('foo');
8+
}
9+
10+
/**
11+
* @param {JQuery} e
12+
*/
13+
doSomethingWithTypes(e) {
14+
e.html('foo');
15+
}
16+
}
17+
18+
new C($('#foo'));
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class MyClass {
2+
foo2(e: JQuery) {
3+
e.html('foo');
4+
}
5+
}
6+
7+
$('#foo');
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import jQuery = require('jquery');
2+
3+
class MyClass {
4+
foo3(e: jQuery) {
5+
e.html('foo');
6+
}
7+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"include": ["."]}

0 commit comments

Comments
 (0)