Clear final global taint frontier
ober
34227c07a107f2dd0caded0082fcad54a59f7afb
--- 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 +268a616 Clear field-sensitive taint frontier 24796db Advance field-sensitive taint parity 971b714 Advance taint exception and field parity 29dd690 Clear taint exact source frontier @@ -69,7 +70,7 @@ make test Result: ```text -186 tests, 186 passed, 0 failed +187 tests, 187 passed, 0 failed ``` Local oracle: @@ -108,6 +109,18 @@ Result: upstream-sweep: 3 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 3 compared ``` +Focused case cleared in this checkpoint: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_final_globals1$' LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + Current first-220 upstream sweep: ```sh @@ -117,7 +130,7 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep MAX_CASES=220 LIST_MISMATCHES=1 M Result: ```text -upstream-sweep: 195 passed, 23 mismatched, 0 jerboa errors, 2 current errors, 220 compared +upstream-sweep: 196 passed, 22 mismatched, 0 jerboa errors, 2 current errors, 220 compared ``` The full 241-case sweep was not rerun after this checkpoint. The last full @@ -138,38 +151,26 @@ errors. ## What Changed In This Checkpoint -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`. +This checkpoint clears `taint_final_globals1`, building on `268a616` which +cleared the first field-sensitive taint frontier. Implementation changes in `src/semgrep/scan.ss`: -- Clean assignment kills are now field-aware instead of token-wide. -- New access-path helpers parse simple member/index paths such as `x`, - `x.a`, `x.c[i].d`, and `params["sql"]`. -- Dynamic bracket indexes normalize to `*`, so `x.c[i].d = safe` can kill - `x.c[j].d` and descendants without killing `x.c[j]`. -- Quoted bracket indexes keep their literal segment, so the earlier - `params["sql"]` exact-source behavior remains preserved. -- Base clean assignments still kill the base and descendants, while member - clean assignments kill only their compatible field subtree. -- 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()`. +- The string-key object pattern helper now supports single-quoted key + metavariables, bare value metavariables, and an optional parent object + property such as `headers: { '$KEY': $H }`. +- The helper still preserves the existing quoted-value behavior: a quoted + metavariable value in the pattern only matches quoted source values. +- Parent object matching is used to keep `headers: { ... }` patterns scoped to + entries inside a `headers` object instead of every quoted key/value entry. +- This lets a focused sink pattern under `fetch(..., { ... })` bind both + `'Content-Type': 'application/json'` and `'Custom': hashvalue`; taint then + reports only the focused value whose labels reach the sink. New smoke coverage in `tests/smoke.ss`: ```text +scan JavaScript taint focused header value under fetch scan Python taint filters impossible exception branches scan taint field source reaches opaque base sink scan taint whole object source honors field cleans @@ -392,12 +393,37 @@ What changed: wider sink argument. This keeps equal focused self-hit cases like `lock(l)` and `f.close()` from reporting on the same call. +`taint_final_globals1` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_final_globals1$' LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh +``` + +What changed: + +- Target file: + `/Users/user/mine/semgrep/tests/rules/taint_final_globals1.js`. +- Shape: + `let hashvalue = window.location.hash.substring(1);` followed by a + `fetch(..., { headers: { 'Custom': hashvalue } })` sink focused to `$H`. +- Jerboa was already finding the source. It missed the sink candidate because + the object key/value helper only handled double-quoted key/value pairs and + did not understand the parent `headers` property. +- The helper now handles `headers: { '$KEY': $H }`, so both header entries are + structural sink candidates and the taint engine reports only the tainted + focused value `hashvalue`. + +Expected matched line for this case is now exactly: + +```text +14 +``` + ## Current First-220 Frontier -The current first-220 sweep has these 23 mismatches: +The current first-220 sweep has these 22 mismatches: ```text -taint_final_globals1 taint_labels2 taint_labels3 taint_lambda2 @@ -438,12 +464,12 @@ vardef_assign_true1 vardef_assign_true2 ``` -## Immediate Next Case: taint_final_globals1 +## Immediate Next Case: taint_labels2 Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_final_globals1$' LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_labels2$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh ``` Current result: @@ -456,73 +482,84 @@ Rule: ```yaml rules: -- id: test +- id: tainting mode: taint - pattern-sources: - - pattern: window.location.hash - pattern-sinks: - - patterns: - - pattern-inside: | - fetch(..., { - ... - }) - - pattern: | - headers: { - '$KEY': $H - } - - focus-metavariable: $H - message: "Semgrep found a match: $KEY $H" languages: - - typescript - - javascript - severity: WARNING + - python + pattern-sinks: + - requires: TAINTED and not CLEANED + pattern: sink(...) + pattern-sources: + - label: TAINTED + pattern: source(...) + - label: CLEANED + pattern: sanitize(...) + message: | + This confirms taint mode works. + severity: ERROR ``` Target: ```text -/Users/user/mine/semgrep/tests/rules/taint_final_globals1.js +/Users/user/mine/semgrep/tests/rules/taint_labels2.py ``` Relevant target: ```text -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: +1 def foo(): +2 a = source() +3 if cond(): +4 b = a +5 b = sanitize() +6 else: +7 b = a +8 #todoruleid: tainting +9 sink(b) +11 def bar(): +12 a = source() +13 if cond(): +14 b = a +15 b = sanitize() +16 #OK: tainting +17 sink(b) +19 def baz(): +20 a = source() +21 if cond(): +22 b = a +23 #ruleid: tainting +24 sink(b) +``` + +Packaged Semgrep expects only: ```text -14 +24 ``` -Jerboa currently reports no findings: +Jerboa currently has a false positive at line 9: ```diff -@@ -1 +0,0 @@ --(finding "test" ".../taint_final_globals1.js" 14 27 ... "WARNING" "Semgrep found a match: 'Custom' hashvalue" "") +@@ -1 +1,2 @@ + (finding "tainting" ".../taint_labels2.py" 24 3 ... "ERROR" "This confirms taint mode works.\n" "") ++(finding "tainting" ".../taint_labels2.py" 9 3 ... "ERROR" "This confirms taint mode works.\n" "") ``` Interpretation for the next fix: -- 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. +- The false positive is caused by label/path imprecision around `if`/`else`. +- Jerboa appears to let the later `else` assignment `b = a` kill or dominate + the `CLEANED` label from the `if` branch, leaving a reachable `TAINTED` label + without `CLEANED` at line 9. +- Semgrep does not report line 9 for this rule. It does report `baz` line 24, + where there is a possible tainted assignment without any cleaning label. +- A likely fix is to make clean/taint assignment kills aware of mutually + exclusive Python `if`/`else` sibling branches, so assignments in one branch do + not kill labels produced in the other branch merely because they occur later + textually. +- Preserve the straight-line behavior in `bar`: `b = sanitize()` after `b = a` + in the same branch should still prevent line 17 from reporting. ## Resolved Earlier Frontier --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -1179,20 +1179,61 @@ match spec) acc))) (reverse acc))))))))) + (def string-key-object-pattern-quoted-value-regex + "[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"'][ \\t]*:[ \\t]*[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"']") + (def string-key-object-pattern-bare-value-regex + "[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"'][ \\t]*:[ \\t]*\\$([A-Za-z_][A-Za-z0-9_]*)") + (def (identifier-token-before-index text index) + (let skip ([i (- index 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) (skip (- i 1))] + [(identifier-token-char? (string-ref text i)) + (let start ([j i]) + (if (and (> j 0) + (identifier-token-char? (string-ref text (- j 1)))) + (start (- j 1)) + (substring text j (+ i 1))))] + [else #f]))) + (def (object-parent-key-before-open text open) + (and open + (let skip ([i (- open 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) (skip (- i 1))] + [(char=? (string-ref text i) #\:) + (identifier-token-before-index text i)] + [else #f])))) (def (string-key-object-pattern-spec pattern) - (let* ([match (re-search - (re "\"\\$([A-Za-z_][A-Za-z0-9_]*)\"[ \\t]*:[ \\t]*\"\\$([A-Za-z_][A-Za-z0-9_]*)\"") - pattern - 0)] + (let* ([quoted-match (re-search + (re string-key-object-pattern-quoted-value-regex) + pattern + 0)] + [bare-match (and (not quoted-match) + (re-search + (re string-key-object-pattern-bare-value-regex) + pattern + 0))] + [match (or quoted-match bare-match)] [key-name (and match (re-match-group match 1))] - [value-name (and match (re-match-group match 2))]) + [value-name (and match (re-match-group match 2))] + [open (and match + (string-find-last-substring-before + pattern + "{" + (re-match-start match)))] + [parent (object-parent-key-before-open pattern open)]) (and key-name value-name (string-find-substring pattern "{") (string-find-substring pattern "}") - (list (cons 'key key-name) (cons 'value value-name))))) + (list + (cons 'key key-name) + (cons 'value value-name) + (cons 'value-quoted? (and quoted-match #t)) + (cons 'parent parent))))) (def string-key-object-regex - "\"[^\"]+\"[ \\t]*:[ \\t]*\"[^\"]*\"") + "[\"'][^\"']+[\"'][ \\t]*:[ \\t]*[^,\\n}]+") (def (string-key-object-finding rule path source match spec) (let* ([key-name (alist-ref/default spec 'key #f)] [value-name (alist-ref/default spec 'value #f)] @@ -1222,13 +1263,19 @@ match-start)] [close (and open (find-matching-close-brace source open))] [start (or open match-start)] - [end (or close (re-match-end match))]) + [end (or close (re-match-end match))] + [parent (alist-ref/default spec 'parent #f)] + [actual-parent (object-parent-key-before-open source open)] + [value-quoted? (alist-ref/default spec 'value-quoted? #f)]) (and key-name value-name key-text value-text key-start value-start + (or (not parent) + (and actual-parent (string=? parent actual-parent))) + (or (not value-quoted?) (quoted-string? value-text)) (let* ([key-binding (make-regex-capture-binding key-name key-text source key-start (+ key-start (string-length key-text)))] --- 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" . "F1C14FB61EA7F0B9") - ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") - ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/scan.ss" . "48ECEC3C7FA6D0BA") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("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 @@ -1270,22 +1270,65 @@ acc))) (reverse acc))))))))) +(def string-key-object-pattern-quoted-value-regex + "[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"'][ \\t]*:[ \\t]*[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"']") + +(def string-key-object-pattern-bare-value-regex + "[\"']\\$([A-Za-z_][A-Za-z0-9_]*)[\"'][ \\t]*:[ \\t]*\\$([A-Za-z_][A-Za-z0-9_]*)") + +(def (identifier-token-before-index text index) + (let skip ([i (- index 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) (skip (- i 1))] + [(identifier-token-char? (string-ref text i)) + (let start ([j i]) + (if (and (> j 0) + (identifier-token-char? (string-ref text (- j 1)))) + (start (- j 1)) + (substring text j (+ i 1))))] + [else #f]))) + +(def (object-parent-key-before-open text open) + (and open + (let skip ([i (- open 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) (skip (- i 1))] + [(char=? (string-ref text i) #\:) + (identifier-token-before-index text i)] + [else #f])))) + (def (string-key-object-pattern-spec pattern) - (let* ([match (re-search - (re "\"\\$([A-Za-z_][A-Za-z0-9_]*)\"[ \\t]*:[ \\t]*\"\\$([A-Za-z_][A-Za-z0-9_]*)\"") - pattern - 0)] + (let* ([quoted-match (re-search + (re string-key-object-pattern-quoted-value-regex) + pattern + 0)] + [bare-match (and (not quoted-match) + (re-search + (re string-key-object-pattern-bare-value-regex) + pattern + 0))] + [match (or quoted-match bare-match)] [key-name (and match (re-match-group match 1))] - [value-name (and match (re-match-group match 2))]) + [value-name (and match (re-match-group match 2))] + [open (and match + (string-find-last-substring-before + pattern + "{" + (re-match-start match)))] + [parent (object-parent-key-before-open pattern open)]) (and key-name value-name (string-find-substring pattern "{") (string-find-substring pattern "}") (list (cons 'key key-name) - (cons 'value value-name))))) + (cons 'value value-name) + (cons 'value-quoted? (and quoted-match #t)) + (cons 'parent parent))))) (def string-key-object-regex - "\"[^\"]+\"[ \\t]*:[ \\t]*\"[^\"]*\"") + "[\"'][^\"']+[\"'][ \\t]*:[ \\t]*[^,\\n}]+") (def (string-key-object-finding rule path source match spec) (let* ([key-name (alist-ref/default spec 'key #f)] @@ -1308,13 +1351,20 @@ [open (string-find-last-substring-before source "{" match-start)] [close (and open (find-matching-close-brace source open))] [start (or open match-start)] - [end (or close (re-match-end match))]) + [end (or close (re-match-end match))] + [parent (alist-ref/default spec 'parent #f)] + [actual-parent (object-parent-key-before-open source open)] + [value-quoted? (alist-ref/default spec 'value-quoted? #f)]) (and key-name value-name key-text value-text key-start value-start + (or (not parent) + (and actual-parent (string=? parent actual-parent))) + (or (not value-quoted?) + (quoted-string? value-text)) (let* ([key-binding (make-regex-capture-binding key-name --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1546,6 +1546,21 @@ (check (finding-start-line (car findings)) => 7) (check (finding-start-col (car findings)) => 18))) +(test-case "scan JavaScript taint focused header value under fetch" + (let* ([taint-config + "rules:\n - id: demo.taint.fetch.header\n mode: taint\n languages: [javascript]\n message: header taint $KEY $H\n severity: WARNING\n pattern-sources:\n - pattern: window.location.hash\n pattern-sinks:\n - patterns:\n - pattern-inside: |\n fetch(..., {\n ...\n })\n - pattern: |\n headers: {\n '$KEY': $H\n }\n - focus-metavariable: $H\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "let hashvalue = window.location.hash.substring(1);\nasync function postData(url = '', data = {}) {\n const response = await fetch(url, {\n headers: {\n 'Content-Type': 'application/json',\n 'Custom': hashvalue,\n }\n });\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 6) + (check (finding-start-col (car findings)) => 17) + (check (finding-message (car findings)) + => "header taint 'Custom' hashvalue"))) + (test-case "scan taint assignment reaches wildcard sink" (let* ([taint-config "rules:\n - id: demo.taint.assignment.wildcard\n mode: taint\n languages: [python]\n message: assigned taint\n severity: WARNING\n pattern-sources:\n - pattern: source()\n pattern-sinks:\n - pattern: sink(...)\n"]