Skip to content

Commit 67cef92

Browse files
committed
JS: Rewrite to use DataFlow::Node API and restrict context
1 parent ddc8f72 commit 67cef92

File tree

3 files changed

+17
-41
lines changed

3 files changed

+17
-41
lines changed

javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -711,41 +711,31 @@ module TaintTracking {
711711
}
712712
}
713713

714+
/**
715+
* Gets a local source of any part of the input to the given stringification `call`.
716+
*/
717+
private DataFlow::Node getAJsonLocalInput(JsonStringifyCall call) {
718+
result = call.getInput()
719+
or
720+
exists(DataFlow::SourceNode source | source = getAJsonLocalInput(call).getALocalSource() |
721+
result = source.getAPropertyWrite().getRhs()
722+
or
723+
result = source.(DataFlow::ObjectLiteralNode).getASpreadProperty()
724+
or
725+
result = source.(DataFlow::ArrayCreationNode).getASpreadArgument()
726+
)
727+
}
728+
714729
/**
715730
* A taint propagating data flow edge arising from JSON unparsing.
716731
*/
717732
private class JsonStringifyTaintStep extends SharedTaintStep {
718733
override predicate serializeStep(DataFlow::Node pred, DataFlow::Node succ) {
719-
exists(JsonStringifyCall call, DataFlow::Node arg |
720-
arg = call.getArgument(0) and
721-
this.findInObject(arg.asExpr(), pred.asExpr()) and
734+
exists(JsonStringifyCall call |
735+
pred = getAJsonLocalInput(call) and
722736
succ = call
723737
)
724738
}
725-
726-
// find target in root object recursively
727-
private predicate findInObject(Expr root, Expr target) {
728-
// base case
729-
root = target
730-
or
731-
// when root is Object
732-
exists(Property property |
733-
root instanceof ObjectExpr and
734-
property = root.(ObjectExpr).getAProperty() and
735-
(
736-
this.findInObject(property.getNameExpr(), target) or
737-
this.findInObject(property.getInit(), target)
738-
)
739-
)
740-
or
741-
// when root is Array
742-
root instanceof ArrayExpr and
743-
this.findInObject(root.(ArrayExpr).getAChildExpr(), target)
744-
or
745-
// when root is VarRef
746-
root instanceof VarRef and
747-
this.findInObject(root.(VarRef).getAVariable().getAnAssignedExpr(), target)
748-
}
749739
}
750740

751741
/**

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,13 +1533,6 @@ edges
15331533
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
15341534
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
15351535
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
1536-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1537-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1538-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1539-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1540-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1541-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1542-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
15431536
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
15441537
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
15451538
| json-stringify.jsx:11:51:11:56 | locale | json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,13 +1583,6 @@ edges
15831583
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
15841584
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
15851585
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:5:9:5:36 | locale |
1586-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1587-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1588-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1589-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1590-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1591-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
1592-
| json-stringify.jsx:5:18:5:36 | req.param("locale") | json-stringify.jsx:31:40:31:61 | JSON.st ... locale) |
15931586
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
15941587
| json-stringify.jsx:11:16:11:58 | `https: ... ocale}` | json-stringify.jsx:35:40:35:61 | JSON.st ... jsonLD) |
15951588
| json-stringify.jsx:11:51:11:56 | locale | json-stringify.jsx:11:16:11:58 | `https: ... ocale}` |

0 commit comments

Comments
 (0)