Clear field-sensitive taint frontier
ober
268a616eb032ff190e833979ca6170b21e75800b
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -31,6 +31,7 @@ taint/dataflow, path and target semantics, autofix, and output schemas. Recent checkpoints before this handoff: ```text +24796db Advance field-sensitive taint parity 971b714 Advance taint exception and field parity 29dd690 Clear taint exact source frontier 7beb039 Advance taint control parity @@ -68,7 +69,7 @@ make test Result: ```text -184 tests, 184 passed, 0 failed +186 tests, 186 passed, 0 failed ``` Local oracle: @@ -83,17 +84,16 @@ Result: oracle: 42 passed, 0 failed ``` -Focused upstream guardrail containing the field-sensitive cases fixed in this -checkpoint: +Focused upstream guardrail containing the field-sensitive cases fixed so far: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_field_sensitive2|taint_field_sensitive4|taint_field_sensitive6|taint_field_sensitive8)$' LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_field_sensitive1|taint_field_sensitive2|taint_field_sensitive3|taint_field_sensitive4|taint_field_sensitive6|taint_field_sensitive7|taint_field_sensitive8)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 4 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 4 compared +upstream-sweep: 7 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 7 compared ``` Nearby regression guardrail: @@ -117,12 +117,11 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep MAX_CASES=220 LIST_MISMATCHES=1 M Result: ```text -upstream-sweep: 193 passed, 25 mismatched, 0 jerboa errors, 2 current errors, 220 compared +upstream-sweep: 195 passed, 23 mismatched, 0 jerboa errors, 2 current errors, 220 compared ``` The full 241-case sweep was not rerun after this checkpoint. The last full -run, before the `taint_exception`, `taint_field_sensitive1`, and current -field-clean fixes, was: +run, before the recent `taint_exception` and field-sensitive fixes, was: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_LIMIT=220 LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh @@ -139,10 +138,10 @@ errors. ## What Changed In This Checkpoint -This checkpoint clears `taint_field_sensitive2`, `taint_field_sensitive4`, -`taint_field_sensitive6`, and `taint_field_sensitive8`, building on `971b714` -which cleared -`taint_exception` and `taint_field_sensitive1`. +This checkpoint clears `taint_field_sensitive3` and `taint_field_sensitive7`, +building on `24796db` which cleared `taint_field_sensitive2`, +`taint_field_sensitive4`, `taint_field_sensitive6`, and +`taint_field_sensitive8`. Implementation changes in `src/semgrep/scan.ss`: @@ -158,6 +157,15 @@ Implementation changes in `src/semgrep/scan.ss`: - Opaque whole-object sinks remain compatible with reachable member taint from `971b714`, so `x.a.i = source()` can still make `sink(x.a)` and `sink(x)` findings. +- Source/sink compatibility now uses the same access-path parser for source + findings and sink arguments. Dynamic bracket segments normalize to `*`, so + `x.d.e[i].f.g[j].h` reaches `x.d.e[k].f.g[l].h` and descendants. +- Same-expression by-side-effect field sources can report a containing sink + when the focused source path is a strict prefix of the sink argument, such as + `x.a` inside `sink(x.a.b)`. +- The same-expression rule deliberately excludes equal focused ranges and + equal access paths, preserving typestate/self-hit behavior such as + `lock(l)` and repeated `f.close()`. New smoke coverage in `tests/smoke.ss`: @@ -166,6 +174,8 @@ scan Python taint filters impossible exception branches scan taint field source reaches opaque base sink scan taint whole object source honors field cleans scan taint ancestor field clean clears descendant sources +scan taint dynamic index field source reaches compatible paths +scan taint by-side-effect field source reaches containing sink ``` The new field-clean test mirrors the upstream `taint_field_sensitive2` shape: @@ -302,6 +312,30 @@ What changed: - This fixed false positives at lines 17, 19, and 21 while preserving the expected pre-clean findings at lines 8, 10, and 12. +`taint_field_sensitive3` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_field_sensitive3$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +``` + +What changed: + +- Target file: + `/Users/user/mine/semgrep/tests/rules/taint_field_sensitive3.js`. +- Jerboa previously missed the dynamic-index exact and descendant sinks at + lines 11 and 13: + `sink(x.d.e[k].f.g[l].h)` and `sink(x.d.e[k].f.g[l].h.i)`. +- The source path `x.d.e[i].f.g[j].h` now parses to an access path with + wildcard dynamic-index segments, so it is compatible with the sink paths + using `k` and `l`. +- Sibling paths remain separated; `sink(x.a.b.d)` and `sink(x.a.c)` stay clean. + +Expected matched lines for this case are now exactly: + +```text +7, 9, 11, 13, 19, 21, 23, 25, 27, 29 +``` + `taint_field_sensitive6` is now cleared: ```sh @@ -337,13 +371,32 @@ What changed: - The later member source `x.a.i = source()` should re-taint `x.a.i`, make the ancestor `x.a` reachable again, and keep `sink(x)` reportable. +`taint_field_sensitive7` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_field_sensitive7$' LIST_MISMATCHES=1 MAX_DIFFS=220 tests/oracle/upstream-sweep.sh +``` + +What changed: + +- Target file: + `/Users/user/mine/semgrep/tests/rules/taint_field_sensitive7.py`. +- Shape: + `x = safe; sink(x.a.b); sink(x.a)` with a by-side-effect source pattern + `$X.b` focused to `$X`. +- Jerboa previously found only the later `sink(x.a)` at line 9. +- Semgrep reports line 5 as well because matching `$X.b` inside `sink(x.a.b)` + focuses and taints `x.a`, and the containing sink argument `x.a.b` can reach + that focused field source. +- The implementation permits this only for strict source-path prefixes inside a + wider sink argument. This keeps equal focused self-hit cases like `lock(l)` + and `f.close()` from reporting on the same call. + ## Current First-220 Frontier -The current first-220 sweep has these 25 mismatches: +The current first-220 sweep has these 23 mismatches: ```text -taint_field_sensitive3 -taint_field_sensitive7 taint_final_globals1 taint_labels2 taint_labels3 @@ -385,12 +438,12 @@ vardef_assign_true1 vardef_assign_true2 ``` -## Immediate Next Case: taint_field_sensitive3 +## Immediate Next Case: taint_final_globals1 Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_field_sensitive3$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_final_globals1$' LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh ``` Current result: @@ -406,10 +459,19 @@ rules: - id: test mode: taint pattern-sources: - - pattern: source + - pattern: window.location.hash pattern-sinks: - - pattern: sink(...) - message: test + - patterns: + - pattern-inside: | + fetch(..., { + ... + }) + - pattern: | + headers: { + '$KEY': $H + } + - focus-metavariable: $H + message: "Semgrep found a match: $KEY $H" languages: - typescript - javascript @@ -419,62 +481,48 @@ rules: Target: ```text -/Users/user/mine/semgrep/tests/rules/taint_field_sensitive3.js +/Users/user/mine/semgrep/tests/rules/taint_final_globals1.js ``` Relevant target: ```text -3 x.a.b.c = source -4 x.d.e[i].f.g[j].h = source -7 sink(x.a.b.c) -9 sink(x.a.b.c.d) -11 sink(x.d.e[k].f.g[l].h) -13 sink(x.d.e[k].f.g[l].h.i) -19 sink(x.a.b) -21 sink(x.a) -23 sink(x.d.e[i].f.g[j]) -25 sink(x.d.e) -27 sink(x.d) -29 sink(x) -33 sink(x.a.b.d) -35 sink(x.a.c) -``` - -Packaged Semgrep expects findings on: +1 let hashvalue = window.location.hash.substring(1); +2 let msg = `Hash value: ${hashvalue}`; +5 async function postData(url = '', data = {}) { +6 const response = await fetch(url, { +11 headers: { +13 //ruleid: test +14 'Custom': hashvalue, +``` + +Packaged Semgrep expects a finding on the focused header value: ```text -7, 9, 11, 13, 19, 21, 23, 25, 27, 29 +14 ``` -Jerboa currently misses only the dynamic-index exact/descendant member sinks: +Jerboa currently reports no findings: ```diff -@@ -1,5 +1,3 @@ --(finding "test" ".../taint_field_sensitive3.js" 11 5 ... "WARNING" "test" "") --(finding "test" ".../taint_field_sensitive3.js" 13 5 ... "WARNING" "test" "") - (finding "test" ".../taint_field_sensitive3.js" 19 5 ... "WARNING" "test" "") - (finding "test" ".../taint_field_sensitive3.js" 21 5 ... "WARNING" "test" "") - (finding "test" ".../taint_field_sensitive3.js" 23 5 ... "WARNING" "test" "") +@@ -1 +0,0 @@ +-(finding "test" ".../taint_final_globals1.js" 14 27 ... "WARNING" "Semgrep found a match: 'Custom' hashvalue" "") ``` Interpretation for the next fix: -- The `x.a.b.c` source path works in both exact and descendant directions: - `sink(x.a.b.c)`, `sink(x.a.b.c.d)`, `sink(x.a.b)`, `sink(x.a)`, and - `sink(x)` are already compatible. -- The dynamic-index path `x.d.e[i].f.g[j].h` works for opaque ancestor sinks: - `sink(x.d.e[i].f.g[j])`, `sink(x.d.e)`, `sink(x.d)`, and `sink(x)` are - already compatible. -- The missing edge is dynamic-index source-to-sink equality/descendant - compatibility for `x.d.e[i].f.g[j].h` to `x.d.e[k].f.g[l].h` and - `x.d.e[k].f.g[l].h.i`. -- Start in the access-path compatibility helpers in `src/semgrep/scan.ss`. - The assignment-kill parser already normalizes dynamic bracket segments to - `*`; source/sink compatibility probably needs the same path parsing instead - of relying only on text token/prefix checks. -- Be careful not to make unrelated sibling paths report. `sink(x.a.b.d)` and - `sink(x.a.c)` must remain clean. +- The likely source flow is: + `window.location.hash` inside `window.location.hash.substring(1)` at line 1 + should taint the assignment target `hashvalue`. +- The sink shape is a focused metavariable inside a nested object literal under + a `fetch(..., { ... })` `pattern-inside`. +- First confirm whether Jerboa is missing the sink candidate or whether the + source fails to propagate through the member-call RHS. If the sink candidate + exists, focus on implicit assignment propagation from a source contained in + the RHS expression to the LHS identifier. +- Preserve the previous guardrail that `source($X)` does not implicitly taint + `$X`; do not solve this by treating all source metavariable bindings as + tainted. ## Resolved Earlier Frontier --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -8276,6 +8276,10 @@ sink source source-text) + (source-access-path-compatible-with-sink? + source + sink + source-text) (and (taint-state-token? source-state) (source-token-in-sink? source @@ -8827,6 +8831,36 @@ source-text)]) (and arg-text (access-path-from-text arg-text))) (access-path-from-text (finding-text sink source-text)))) + (def (access-path-reachable? source-path sink-path) + (or (access-path-prefix? source-path sink-path) + (access-path-prefix? sink-path source-path))) + (def (access-path-strict-prefix? prefix path) + (and (access-path-prefix? prefix path) + (not (access-path-equal? prefix path)))) + (def (source-access-paths source source-text) + (let ([path (access-path-from-text + (finding-text source source-text))]) + (if path (list path) '()))) + (def (source-access-path-compatible-with-sink? + source + sink + source-text) + (let ([sink-path (sink-access-path sink source-text)]) + (and sink-path + (any? + (lambda (source-path) + (access-path-reachable? source-path sink-path)) + (source-access-paths source source-text))))) + (def (source-access-path-strict-prefix-of-sink? + source + sink + source-text) + (let ([sink-path (sink-access-path sink source-text)]) + (and sink-path + (any? + (lambda (source-path) + (access-path-strict-prefix? source-path sink-path)) + (source-access-paths source source-text))))) (def (assignment-path-kills-source? source assignment-finding @@ -8967,6 +9001,13 @@ (source-state-before-sink? source-state source sink) (and (taint-state-contained? source-state) (finding-range-contains? sink source)) + (and (taint-state-token? source-state) + (finding-range-contains? sink source) + (not (finding-range-equal? sink source)) + (source-access-path-strict-prefix-of-sink? + source + sink + source-text)) (and (not (taint-state-token? source-state)) (direct-call-argument-source? source --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,11 +3,11 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "7E5441BD00A7F1D4") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "EB4346CDD34F7AE7") - ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/scan.ss" . "F1C14FB61EA7F0B9") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") - ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/rule.ss" . "E12C108153C181FA") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") + ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/match/structural.ss" . "F7B63A9A6FA028B") ("src/semgrep/main.ss" . "A4EC9E7F2A09D25E") ("src/semgrep/cli.ss" . "D56FC2D2EB449BA6")) --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -8785,6 +8785,10 @@ (finding-range-contains? sink source)) (finding-text-equals-any-binding? source sink source-text) (finding-text-equals-any-binding? sink source source-text) + (source-access-path-compatible-with-sink? + source + sink + source-text) (and (taint-state-token? source-state) (source-token-in-sink? source sink source-text)) (field-source-taints-base-sink? @@ -9299,6 +9303,32 @@ (and arg-text (access-path-from-text arg-text))) (access-path-from-text (finding-text sink source-text)))) +(def (access-path-reachable? source-path sink-path) + (or (access-path-prefix? source-path sink-path) + (access-path-prefix? sink-path source-path))) + +(def (access-path-strict-prefix? prefix path) + (and (access-path-prefix? prefix path) + (not (access-path-equal? prefix path)))) + +(def (source-access-paths source source-text) + (let ([path (access-path-from-text (finding-text source source-text))]) + (if path (list path) '()))) + +(def (source-access-path-compatible-with-sink? source sink source-text) + (let ([sink-path (sink-access-path sink source-text)]) + (and sink-path + (any? (lambda (source-path) + (access-path-reachable? source-path sink-path)) + (source-access-paths source source-text))))) + +(def (source-access-path-strict-prefix-of-sink? source sink source-text) + (let ([sink-path (sink-access-path sink source-text)]) + (and sink-path + (any? (lambda (source-path) + (access-path-strict-prefix? source-path sink-path)) + (source-access-paths source source-text))))) + (def (assignment-path-kills-source? source assignment-finding sink source-text) (let* ([lhs (assignment-lhs-text assignment-finding source-text)] [source-path (access-path-from-text (finding-text source source-text))] @@ -9418,6 +9448,13 @@ (source-state-before-sink? source-state source sink) (and (taint-state-contained? source-state) (finding-range-contains? sink source)) + (and (taint-state-token? source-state) + (finding-range-contains? sink source) + (not (finding-range-equal? sink source)) + (source-access-path-strict-prefix-of-sink? + source + sink + source-text)) (and (not (taint-state-token? source-state)) (direct-call-argument-source? source sink source-text))) (not (and (taint-assume-safe-booleans? rule) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1650,6 +1650,35 @@ (check (finding-start-line (cadr findings)) => 5) (check (finding-start-line (caddr findings)) => 6))) +(test-case "scan taint dynamic index field source reaches compatible paths" + (let* ([taint-config + "rules:\n - id: demo.taint.field-dynamic-index\n mode: taint\n languages: [javascript]\n message: dynamic field taint\n severity: WARNING\n pattern-sources:\n - pattern: source\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function f() {\n x.d.e[i].f.g[j].h = source;\n sink(x.d.e[k].f.g[l].h);\n sink(x.d.e[k].f.g[l].h.i);\n sink(x.d.e[k].f.g[l]);\n sink(x.d.e);\n sink(x.d);\n sink(x.d.e[k].f.g[l].q);\n}\n")]) + (check (length findings) => 5) + (check (finding-start-line (car findings)) => 3) + (check (finding-start-line (cadr findings)) => 4) + (check (finding-start-line (caddr findings)) => 5) + (check (finding-start-line (cadddr findings)) => 6) + (check (finding-start-line (car (cddddr findings))) => 7))) + +(test-case "scan taint by-side-effect field source reaches containing sink" + (let* ([taint-config + "rules:\n - id: demo.taint.field-side-effect-containing\n mode: taint\n languages: [python]\n message: field side-effect taint\n severity: WARNING\n pattern-sources:\n - by-side-effect: true\n patterns:\n - pattern: $X.b\n - focus-metavariable: $X\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "python" + "demo.py" + "x = safe\nsink(x.a.b)\nsink(x.a)\n")]) + (check (length findings) => 2) + (check (finding-start-line (car findings)) => 2) + (check (finding-start-line (cadr findings)) => 3))) + (test-case "scan taint by-side-effect source reaches wildcard sink" (let* ([taint-config "rules:\n - id: demo.taint.by-side-effect-source\n mode: taint\n languages: [python]\n message: side-effect taint\n severity: WARNING\n pattern-sources:\n - by-side-effect: true\n patterns:\n - pattern: $X = source()\n - focus-metavariable: $X\n pattern-sinks:\n - pattern: sink(...)\n"]