Merge pull request #1331 from asger-semmle/destructuring-assignment-fix

Approved by xiemaisi
This commit is contained in:
semmle-qlci
2019-05-21 11:32:36 +01:00
committed by GitHub
5 changed files with 110 additions and 1 deletions

View File

@@ -1143,6 +1143,23 @@ module DataFlow {
succ = TDestructuringPatternNode(def.getTarget())
)
or
// flow from the value read from a property pattern to the value being
// destructured in the child pattern. For example, for
//
// let { p: { q: x } } = obj
//
// add edge from the 'p:' pattern to '{ q:x }'.
exists(PropertyPattern pattern |
pred = TPropNode(pattern) and
succ = TDestructuringPatternNode(pattern.getValuePattern())
)
or
// Like the step above, but for array destructuring patterns.
exists(Expr elm |
pred = TElementPatternNode(_, elm) and
succ = TDestructuringPatternNode(elm)
)
or
// flow from 'this' parameter into 'this' expressions
exists(ThisExpr thiz |
pred = TThisNode(thiz.getBindingContainer()) and
@@ -1154,7 +1171,7 @@ module DataFlow {
* Holds if there is a step from `pred` to `succ` through a field accessed through `this` in a class.
*/
predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassNode cls, string prop |
exists(ClassNode cls, string prop |
pred = cls.getAReceiverNode().getAPropertyWrite(prop).getRhs() and
succ = cls.getAReceiverNode().getAPropertyRead(prop)
)

View File

@@ -22,6 +22,9 @@
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
| destruct.js:15:7:15:14 | source() | destruct.js:5:10:5:10 | z |
| destruct.js:15:7:15:14 | source() | destruct.js:8:10:8:10 | w |
| destruct.js:15:7:15:14 | source() | destruct.js:11:10:11:10 | q |
| exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
| exceptions.js:21:17:21:24 | source() | exceptions.js:23:10:23:10 | e |
| exceptions.js:21:17:21:24 | source() | exceptions.js:24:10:24:21 | e.toString() |

View File

@@ -0,0 +1,44 @@
| access-path-sanitizer.js:2:18:2:25 | source() | access-path-sanitizer.js:4:8:4:12 | obj.x |
| advanced-callgraph.js:2:13:2:20 | source() | advanced-callgraph.js:6:22:6:22 | v |
| callbacks.js:4:6:4:13 | source() | callbacks.js:34:27:34:27 | x |
| callbacks.js:4:6:4:13 | source() | callbacks.js:35:27:35:27 | x |
| callbacks.js:5:6:5:13 | source() | callbacks.js:34:27:34:27 | x |
| callbacks.js:5:6:5:13 | source() | callbacks.js:35:27:35:27 | x |
| callbacks.js:25:16:25:23 | source() | callbacks.js:47:26:47:26 | x |
| callbacks.js:25:16:25:23 | source() | callbacks.js:48:26:48:26 | x |
| callbacks.js:37:17:37:24 | source() | callbacks.js:37:37:37:37 | x |
| callbacks.js:44:17:44:24 | source() | callbacks.js:41:10:41:10 | x |
| callbacks.js:50:18:50:25 | source() | callbacks.js:30:29:30:29 | y |
| callbacks.js:51:18:51:25 | source() | callbacks.js:30:29:30:29 | y |
| captured-sanitizer.js:25:3:25:10 | source() | captured-sanitizer.js:15:10:15:10 | x |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:18:8:18:14 | c.taint |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:22:8:22:19 | c_safe.taint |
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:26:8:26:14 | d.taint |
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
| exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
| exceptions.js:48:16:48:23 | source() | exceptions.js:50:10:50:10 | e |
| exceptions.js:59:24:59:31 | source() | exceptions.js:61:12:61:12 | e |
| exceptions.js:88:6:88:13 | source() | exceptions.js:11:10:11:10 | e |
| exceptions.js:93:11:93:18 | source() | exceptions.js:95:10:95:10 | e |
| exceptions.js:100:13:100:20 | source() | exceptions.js:102:12:102:12 | e |
| exceptions.js:115:21:115:28 | source() | exceptions.js:121:10:121:10 | e |
| exceptions.js:144:9:144:16 | source() | exceptions.js:129:10:129:10 | e |
| exceptions.js:144:9:144:16 | source() | exceptions.js:132:8:132:27 | returnThrownSource() |
| exceptions.js:150:13:150:20 | source() | exceptions.js:153:10:153:10 | e |
| exceptions.js:158:13:158:20 | source() | exceptions.js:161:10:161:10 | e |
| indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
| indexOf.js:4:11:4:18 | source() | indexOf.js:13:10:13:10 | x |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:41:10:41:18 | id(taint) |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
| sanitizer-guards.js:2:11:2:18 | source() | sanitizer-guards.js:4:8:4:8 | x |
| sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:15:10:15:15 | this.x |
| sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:21:14:21:19 | this.x |
| sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:26:9:26:14 | this.x |
| thisAssignments.js:4:17:4:24 | source() | thisAssignments.js:5:10:5:18 | obj.field |
| thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
| tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |

View File

@@ -0,0 +1,27 @@
import javascript
DataFlow::CallNode getACall(string name) { result.getCalleeName() = name }
class BasicConfig extends DataFlow::Configuration {
BasicConfig() { this = "BasicConfig" }
override predicate isSource(DataFlow::Node node) { node = getACall("source") }
override predicate isSink(DataFlow::Node node) { node = getACall("sink").getAnArgument() }
override predicate isBarrierGuard(DataFlow::BarrierGuardNode node) {
node instanceof BasicBarrierGuard
}
}
class BasicBarrierGuard extends DataFlow::BarrierGuardNode, DataFlow::CallNode {
BasicBarrierGuard() { this = getACall("isSafe") }
override predicate blocks(boolean outcome, Expr e) {
outcome = true and e = getArgument(0).asExpr()
}
}
from BasicConfig cfg, DataFlow::Node src, DataFlow::Node sink
where cfg.hasFlow(src, sink)
select src, sink

View File

@@ -0,0 +1,18 @@
function test() {
function f(obj) {
let { x: { y: { z } } } = obj;
sink(z); // NOT OK
let [[[w]]] = obj;
sink(w); // NOT OK
let { x: [ { y: q } ] } = obj;
sink(q); // NOT OK
}
function g() {
f(source());
}
}