Skip to content

Commit 92a6812

Browse files
asgerfasgerf
committed
JS: Step through jQuery callback return values
1 parent bc2a772 commit 92a6812

File tree

5 files changed

+39
-1
lines changed

5 files changed

+39
-1
lines changed

‎javascript/ql/lib/semmle/javascript/frameworks/jQuery.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,25 @@ module JQuery {
563563
}
564564
}
565565

566+
/** Gets a data flow node that reaches a sink that is interpreted as HTML. */
567+
private DataFlow::SourceNode htmlCallback(DataFlow::TypeBackTracker t) {
568+
t.start() and
569+
any(JQuery::MethodCall c).interpretsArgumentAsHtml(result.getALocalUse())
570+
or
571+
exists(DataFlow::TypeBackTracker t2 | result = htmlCallback(t2).backtrack(t2, t))
572+
}
573+
574+
/**
575+
* Gets a function that is passed as a callback to a jQuery function, which will interpret its return value as HTML.
576+
*
577+
* For example, this gets the function `f` below:
578+
* ```js
579+
* function f() { ... }
580+
* $('#foo').replaceWith(f);
581+
* ```
582+
*/
583+
DataFlow::FunctionNode htmlCallback() { result = htmlCallback(DataFlow::TypeBackTracker::end()) }
584+
566585
/**
567586
* Holds for jQuery plugin definitions of the form `$.fn.<pluginName> = <plugin>` or `$.extend($.fn, {<pluginName>, <plugin>})`.
568587
*/

‎javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ class Configuration extends TaintTracking::Configuration {
122122
TaintedUrlSuffix::step(src, trg, TaintedUrlSuffix::label(), DataFlow::FlowLabel::taint()) and
123123
inlbl = TaintedUrlSuffix::label() and
124124
outlbl = prefixLabel()
125+
or
126+
exists(DataFlow::FunctionNode callback |
127+
callback = JQuery::htmlCallback() and
128+
src = callback.getReturnNode() and
129+
trg = callback and
130+
inlbl = outlbl
131+
)
125132
}
126133
}
127134

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
| query-tests/Security/CWE-079/DomBasedXss/jquery.js:37 | expected an alert, but found none | NOT OK | |

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,9 @@ nodes
433433
| jquery.js:34:13:34:16 | hash |
434434
| jquery.js:36:25:36:31 | tainted |
435435
| jquery.js:36:25:36:31 | tainted |
436+
| jquery.js:37:25:37:37 | () => tainted |
437+
| jquery.js:37:25:37:37 | () => tainted |
438+
| jquery.js:37:31:37:37 | tainted |
436439
| json-stringify.jsx:5:9:5:36 | locale |
437440
| json-stringify.jsx:5:9:5:36 | locale |
438441
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
@@ -1516,6 +1519,7 @@ edges
15161519
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
15171520
| jquery.js:2:7:2:40 | tainted | jquery.js:36:25:36:31 | tainted |
15181521
| jquery.js:2:7:2:40 | tainted | jquery.js:36:25:36:31 | tainted |
1522+
| jquery.js:2:7:2:40 | tainted | jquery.js:37:31:37:37 | tainted |
15191523
| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted |
15201524
| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted |
15211525
| jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "<div i ... + "\\">" |
@@ -1569,6 +1573,8 @@ edges
15691573
| jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') |
15701574
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
15711575
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
1576+
| jquery.js:37:31:37:37 | tainted | jquery.js:37:25:37:37 | () => tainted |
1577+
| jquery.js:37:31:37:37 | tainted | jquery.js:37:25:37:37 | () => tainted |
15721578
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:11:51:11:56 | locale |
15731579
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:19:56:19:61 | locale |
15741580
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |
@@ -2360,6 +2366,7 @@ edges
23602366
| jquery.js:28:5:28:43 | window. ... ?', '') | jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') | Cross-site scripting vulnerability due to $@. | jquery.js:28:5:28:26 | window. ... .search | user-provided value |
23612367
| jquery.js:34:5:34:25 | '<b>' + ... '</b>' | jquery.js:18:14:18:33 | window.location.hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | jquery.js:18:14:18:33 | window.location.hash | user-provided value |
23622368
| jquery.js:36:25:36:31 | tainted | jquery.js:2:17:2:40 | documen ... .search | jquery.js:36:25:36:31 | tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
2369+
| jquery.js:37:25:37:37 | () => tainted | jquery.js:2:17:2:40 | documen ... .search | jquery.js:37:25:37:37 | () => tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
23632370
| json-stringify.jsx:31:40:31:61 | JSON.st ... locale) | json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) | Cross-site scripting vulnerability due to $@. | json-stringify.jsx:5:18:5:36 | req.param("locale") | user-provided value |
23642371
| json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) | json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) | Cross-site scripting vulnerability due to $@. | json-stringify.jsx:5:18:5:36 | req.param("locale") | user-provided value |
23652372
| jwt-server.js:11:19:11:29 | decoded.foo | jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:11:19:11:29 | decoded.foo | Cross-site scripting vulnerability due to $@. | jwt-server.js:7:17:7:35 | req.param("wobble") | user-provided value |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,9 @@ nodes
433433
| jquery.js:34:13:34:16 | hash |
434434
| jquery.js:36:25:36:31 | tainted |
435435
| jquery.js:36:25:36:31 | tainted |
436+
| jquery.js:37:25:37:37 | () => tainted |
437+
| jquery.js:37:25:37:37 | () => tainted |
438+
| jquery.js:37:31:37:37 | tainted |
436439
| json-stringify.jsx:5:9:5:36 | locale |
437440
| json-stringify.jsx:5:9:5:36 | locale |
438441
| json-stringify.jsx:5:18:5:36 | req.param("locale") |
@@ -1566,6 +1569,7 @@ edges
15661569
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
15671570
| jquery.js:2:7:2:40 | tainted | jquery.js:36:25:36:31 | tainted |
15681571
| jquery.js:2:7:2:40 | tainted | jquery.js:36:25:36:31 | tainted |
1572+
| jquery.js:2:7:2:40 | tainted | jquery.js:37:31:37:37 | tainted |
15691573
| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted |
15701574
| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted |
15711575
| jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "<div i ... + "\\">" |
@@ -1619,6 +1623,8 @@ edges
16191623
| jquery.js:28:5:28:26 | window. ... .search | jquery.js:28:5:28:43 | window. ... ?', '') |
16201624
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
16211625
| jquery.js:34:13:34:16 | hash | jquery.js:34:5:34:25 | '<b>' + ... '</b>' |
1626+
| jquery.js:37:31:37:37 | tainted | jquery.js:37:25:37:37 | () => tainted |
1627+
| jquery.js:37:31:37:37 | tainted | jquery.js:37:25:37:37 | () => tainted |
16221628
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:11:51:11:56 | locale |
16231629
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:19:56:19:61 | locale |
16241630
| json-stringify.jsx:5:9:5:36 | locale | json-stringify.jsx:31:55:31:60 | locale |

0 commit comments

Comments
 (0)