hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 17:25:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Add imprecise post-update steps for when a captured var/this is not tracked precisely

Browse Source 
With the capture library we sometimes bails out of handling certain functions for scalability reasons.

This means we have a notion of "captured but imprecisely-tracked" variables and 'this'. In these cases we go back to propagating flow from a post-update node to the local source.
This commit is contained in:
Asger F
2024-10-23 11:01:46 +02:00
parent d557c7689c
commit 1b85feb1fa
4 changed files with 31 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll
View File

@@ -1049,6 +1049,28 @@ private predicate isThisNodeTrackedByVariableCapture(DataFlow::ThisNode thisNode
)
}
/**
* Holds if there should be flow from `postUpdate` to `target` because of a variable/this value
* that is captured but not tracked precisely by the variable-capture library.
*/
pragma[nomagic]
private predicate imprecisePostUpdateStep(DataFlow::PostUpdateNode postUpdate, DataFlow::Node target) {
exists(LocalVariableOrThis var, DataFlow::Node use |
// 'var' is captured but not tracked precisely
var.isCaptured() and
not var instanceof VariableCaptureConfig::CapturedVariable and
(
use = TValueNode(var.asLocalVariable().getAnAccess())
or
use = TValueNode(var.getAThisExpr())
or
use = TImplicitThisUse(var.getAThisUse(), false)
) and
postUpdate.getPreUpdateNode() = use and
target = use.getALocalSource()
)
}
/**
* Holds if there is a value-preserving steps `node1` -> `node2` that might
* be cross function boundaries.
@@ -1059,6 +1081,8 @@ private predicate valuePreservingStep(Node node1, Node node2) {
not isBlockedLegacyNode(node2) and
not isThisNodeTrackedByVariableCapture(node1)
or
imprecisePostUpdateStep(node1, node2)
or
FlowSteps::propertyFlowStep(node1, node2)
or
FlowSteps::globalFlowStep(node1, node2)

6
javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
View File

@@ -17,9 +17,6 @@ legacyDataFlowDifference
| callbacks.js:44:17:44:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |
| capture-flow.js:89:13:89:20 | source() | capture-flow.js:89:6:89:21 | test3c(source()) | only flow with NEW data flow library |
| capture-flow.js:101:12:101:19 | source() | capture-flow.js:102:6:102:20 | test5("safe")() | only flow with OLD data flow library |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo | only flow with OLD data flow library |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:274:6:274:45 | new Cap ... ()).foo | only flow with OLD data flow library |
| capture-flow.js:283:34:283:41 | source() | capture-flow.js:284:6:284:44 | new Cap ... e').foo | only flow with NEW data flow library |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:40:8:40:14 | e.taint | only flow with NEW data flow library |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:44:8:44:19 | f_safe.taint | only flow with NEW data flow library |
| constructor-calls.js:20:15:20:22 | source() | constructor-calls.js:39:8:39:14 | e.param | only flow with NEW data flow library |
@@ -126,8 +123,9 @@ flow
| capture-flow.js:259:23:259:30 | source() | capture-flow.js:252:14:252:36 | objectW ... s.field |
| capture-flow.js:259:23:259:30 | source() | capture-flow.js:253:14:253:23 | this.field |
| capture-flow.js:262:16:262:23 | source() | capture-flow.js:264:14:264:21 | this.foo |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:274:6:274:45 | new Cap ... ()).foo |
| capture-flow.js:283:34:283:41 | source() | capture-flow.js:283:6:283:46 | new Cap ... ()).foo |
| capture-flow.js:283:34:283:41 | source() | capture-flow.js:284:6:284:44 | new Cap ... e').foo |
| captured-sanitizer.js:25:3:25:10 | source() | captured-sanitizer.js:15:10:15:10 | x |
| case.js:2:16:2:23 | source() | case.js:5:8:5:35 | changeC ... source) |
| case.js:2:16:2:23 | source() | case.js:8:8:8:24 | camelCase(source) |

4
javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
View File

@@ -11,8 +11,6 @@ legacyDataFlowDifference
| callbacks.js:44:17:44:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |
| capture-flow.js:89:13:89:20 | source() | capture-flow.js:89:6:89:21 | test3c(source()) | only flow with NEW data flow library |
| capture-flow.js:101:12:101:19 | source() | capture-flow.js:102:6:102:20 | test5("safe")() | only flow with OLD data flow library |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo | only flow with OLD data flow library |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:274:6:274:45 | new Cap ... ()).foo | only flow with OLD data flow library |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:40:8:40:14 | e.taint | only flow with NEW data flow library |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:44:8:44:19 | f_safe.taint | only flow with NEW data flow library |
| constructor-calls.js:20:15:20:22 | source() | constructor-calls.js:39:8:39:14 | e.param | only flow with NEW data flow library |
@@ -100,6 +98,8 @@ flow
| capture-flow.js:259:23:259:30 | source() | capture-flow.js:252:14:252:36 | objectW ... s.field |
| capture-flow.js:259:23:259:30 | source() | capture-flow.js:253:14:253:23 | this.field |
| capture-flow.js:262:16:262:23 | source() | capture-flow.js:264:14:264:21 | this.foo |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo |
| capture-flow.js:274:33:274:40 | source() | capture-flow.js:274:6:274:45 | new Cap ... ()).foo |
| capture-flow.js:283:34:283:41 | source() | capture-flow.js:283:6:283:46 | new Cap ... ()).foo |
| 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:24:8:24:14 | c.taint |

6
javascript/ql/test/library-tests/TaintTracking/capture-flow.js
View File

@@ -269,9 +269,9 @@ function CaptureThisWithoutJump(x) {
[1].forEach(() => {
this.foo = x;
});
sink(this.foo); // NOT OK [INCONSISTENCY]
sink(this.foo); // NOT OK
}
sink(new CaptureThisWithoutJump(source()).foo); // NOT OK [INCONSISTENCY]
sink(new CaptureThisWithoutJump(source()).foo); // NOT OK
sink(new CaptureThisWithoutJump('safe').foo); // OK
function CaptureThisWithoutJump2(x) {
@@ -281,4 +281,4 @@ function CaptureThisWithoutJump2(x) {
return y;
}
sink(new CaptureThisWithoutJump2(source()).foo); // NOT OK
sink(new CaptureThisWithoutJump2('safe').foo); // OK [INCONSISTENCY]
sink(new CaptureThisWithoutJump2('safe').foo); // OK