From 2a55ba5d4f9f9cb81999d1db032e1a691153b8e2 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Thu, 2 Jan 2020 09:48:57 +0000 Subject: [PATCH 1/2] JavaScript: Fix join order in `PathNode.getASuccessor`. --- .../semmle/javascript/dataflow/Configuration.qll | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll index 9e61c627cdf4..63831e284700 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll @@ -1070,6 +1070,18 @@ private MidPathNode finalMidNode(SinkPathNode snk) { ) } +/** + * Holds if `nd` is a mid node wrapping `(predNd, cfg, summary)`, and there is a flow step + * from `predNd` to `succNd` under `cfg` with summary `newSummary`. + * + * This helper predicate exists to clarify the intended join order in `getASuccessor` below. + */ +pragma[noinline] +private predicate midNodeStep(PathNode nd, DataFlow::Node predNd, Configuration cfg, PathSummary summary, DataFlow::Node succNd, PathSummary newSummary) { + nd = MkMidNode(predNd, cfg, summary) and + flowStep(predNd, id(cfg), succNd, newSummary) +} + /** * Gets a node to which data from `nd` may flow in one step. */ @@ -1079,8 +1091,7 @@ private PathNode getASuccessor(PathNode nd) { or // mid node to mid node exists(Configuration cfg, DataFlow::Node predNd, PathSummary summary, DataFlow::Node succNd, PathSummary newSummary | - nd = MkMidNode(predNd, cfg, summary) and - flowStep(predNd, id(cfg), succNd, newSummary) and + midNodeStep(nd, predNd, cfg, summary, succNd, newSummary) and result = MkMidNode(succNd, id(cfg), summary.append(newSummary)) ) or From de02bb4a0d125b0d7a8911c6a5963492c374f540 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Thu, 2 Jan 2020 09:49:09 +0000 Subject: [PATCH 2/2] JavaScript: Prevent joining on configuration in `onPath`. --- .../ql/src/semmle/javascript/dataflow/Configuration.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll index 63831e284700..43fd3473d7fd 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll @@ -938,8 +938,8 @@ private predicate onPath(DataFlow::Node nd, DataFlow::Configuration cfg, PathSum or exists(DataFlow::Node mid, PathSummary stepSummary | reachableFromSource(nd, cfg, summary) and - flowStep(nd, cfg, mid, stepSummary) and - onPath(mid, cfg, summary.append(stepSummary)) + flowStep(nd, id(cfg), mid, stepSummary) and + onPath(mid, id(cfg), summary.append(stepSummary)) ) }