Honor taint metavariable unification
ober
113620a3b866bf5942a70d2abac2bfeae9e517d1
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,10 +1,10 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-29 02:40 MDT +Date: 2026-05-29 02:51 MDT Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` -Current pre-handoff HEAD: `9b67df7 Honor safe comparisons taint option` +Current pre-handoff HEAD: `9ed0adb Clear React prop spread taint frontier` The user wants this project carried forward until the pure Jerboa port reaches Semgrep parity. Continue from this state. Do not restart broad discovery from @@ -52,7 +52,7 @@ make test Result: ```text -207 tests, 207 passed, 0 failed +208 tests, 208 passed, 0 failed ``` Local oracle: @@ -67,10 +67,10 @@ Result: oracle: 42 passed, 0 failed ``` -Focused `taint_spread_record_big` case: +Focused `taint_unify_mvars` case: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_spread_record_big$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_unify_mvars$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh ``` Result: @@ -82,17 +82,16 @@ upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 com Nearby taint guardrail: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_spread_record_big|taint_unify_mvars|taint_safe_comparisons|taint_react|taint_param_source3)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_unify_mvars|taint_spread_record_big|taint_safe_comparisons|taint_react)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 4 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 5 compared +upstream-sweep: 4 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 4 compared ``` -The remaining mismatch in that guardrail is the known wider frontier -`taint_unify_mvars`. +The next mismatch is outside this taint guardrail: `ts_decorated_async_property`. Full upstream sweep: @@ -103,7 +102,7 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep LIST_MISMATCHES=1 MAX_DIFFS=0 tes Result: ```text -upstream-sweep: 230 passed, 9 mismatched, 0 jerboa errors, 2 current errors, 241 compared +upstream-sweep: 231 passed, 8 mismatched, 0 jerboa errors, 2 current errors, 241 compared ``` The two current errors are packaged-Semgrep oracle errors, not Jerboa scanner @@ -112,40 +111,37 @@ the command above intentionally ran the full sorted 241-case sweep. ## What Changed In This Checkpoint -This checkpoint clears upstream `taint_spread_record_big`, reducing the full sweep +This checkpoint clears upstream `taint_unify_mvars`, reducing the full sweep frontier from: ```text -229 passed, 10 mismatched, 0 jerboa errors, 2 current errors +230 passed, 9 mismatched, 0 jerboa errors, 2 current errors ``` to: ```text -230 passed, 9 mismatched, 0 jerboa errors, 2 current errors +231 passed, 8 mismatched, 0 jerboa errors, 2 current errors ``` Implementation changes in `src/semgrep/scan.ss`: -- Added focused JavaScript scanners for upstream React prop-injection shapes: - destructured `this.props` sources, function-parameter `props` sources, and - `React.createElement(..., {...$PROPS...}, ...)` object-spread sinks. -- The destructuring scanner binds each object field in - `const { i18n, text, attachments } = this.props`. -- The function source scanner binds the parameter in function expressions such - as `function (props) { ... }`. -- The spread sink scanner binds identifiers in object spreads such as - `{...props}` inside `React.createElement`. +- Added `taint-unify-mvars?` for the Semgrep rule option + `taint_unify_mvars`. +- Added `taint-mvars-compatible?`, which requires source and sink findings to + have compatible shared metavariable bindings when that option is enabled. +- Applied the compatibility check inside `source-state-reaches-sink-spec-shape?` + before normal taint reachability checks. New smoke coverage in `tests/smoke.ss`: ```text -scan JavaScript taint React createElement prop injection +scan JavaScript taint unifies source and sink metavariables ``` ## Recently Cleared Cases -Clean in the five most recent checkpoints: +Clean in the recent taint checkpoints: ```text taint_param_source1 @@ -155,6 +151,7 @@ taint_propagator2 taint_react taint_safe_comparisons taint_spread_record_big +taint_unify_mvars ``` Relevant upstream files: @@ -174,33 +171,32 @@ Relevant upstream files: /Users/user/mine/semgrep/tests/rules/taint_safe_comparisons.py /Users/user/mine/semgrep/tests/rules/taint_spread_record_big.yaml /Users/user/mine/semgrep/tests/rules/taint_spread_record_big.js +/Users/user/mine/semgrep/tests/rules/taint_unify_mvars.yaml +/Users/user/mine/semgrep/tests/rules/taint_unify_mvars.js ``` -Important isolation finding for `taint_spread_record_big`: +Important isolation finding for `taint_unify_mvars`: -- Before this checkpoint: - - the dangerous `dangerouslySetInnerHTML: {__html: $PROPS}` sink formula - already produced candidates for `text`, `comment`, and sanitized calls; - - source formulas returned zero candidates; - - the object-spread sink formula returned zero candidates. -- After the patch, the focused upstream probe produced exactly: +- Source findings already carried `$FUNC` from + `pattern-inside: function $FUNC(...) { ... }`: ```text - line 11: text - line 31: comment - line 38: props + source() in foo has FUNC=foo + source() in bar has FUNC=bar ``` -- The sanitizer cases at lines 45 and 52 remain clean through existing - sanitizer handling once `comment` and `props` propagation are available. +- Sink findings already carried `$FUNC` and `$TAINT` from + `pattern-inside: $FUNC($TAINT)`. +- The missing behavior was honoring `taint_unify_mvars: true` during source to + sink reachability. Without that, `source()` inside `foo` could reach + `bar(x)` and vice versa. ## Current Full-Sweep Frontier The current full sorted upstream sweep compared 241 rule/target pairs and has -these 9 mismatches: +these 8 mismatches: ```text -taint_unify_mvars ts_decorated_async_property ts_decorated_static_property unicode_string @@ -217,25 +213,25 @@ Use this command to refresh the full list: SEMGREP_CURRENT=/Users/user/.local/bin/semgrep LIST_MISMATCHES=1 MAX_DIFFS=0 tests/oracle/upstream-sweep.sh ``` -## Recommended Next Target: taint_unify_mvars +## Recommended Next Target: ts_decorated_async_property -The next sorted frontier is `taint_unify_mvars`. +The next sorted frontier is `ts_decorated_async_property`. Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_unify_mvars$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^ts_decorated_async_property$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh ``` Current result: ```text -MISMATCH taint_unify_mvars -@@ -1,2 +1,4 @@ -+(finding "test" ".../taint_unify_mvars.js" 12 9 150 12 10 151 "WARNING" "Test" "") - (finding "test" ".../taint_unify_mvars.js" 14 9 180 14 10 181 "WARNING" "Test" "") - (finding "test" ".../taint_unify_mvars.js" 4 9 61 4 10 62 "WARNING" "Test" "") -+(finding "test" ".../taint_unify_mvars.js" 6 9 87 6 10 88 "WARNING" "Test" "") +MISMATCH ts_decorated_async_property +@@ -1,4 +0,0 @@ +-(finding "decorated-async-property" ".../ts_decorated_async_property.ts" 12 3 122 13 14 144 "ERROR" "Matching a decorated async property" "") +-(finding "decorated-async-property" ".../ts_decorated_async_property.ts" 17 3 191 18 21 220 "ERROR" "Matching a decorated async property" "") +-(finding "decorated-async-property" ".../ts_decorated_async_property.ts" 22 3 267 24 4 294 "ERROR" "Matching a decorated async property" "") +-(finding "decorated-async-property" ".../ts_decorated_async_property.ts" 27 3 336 29 4 370 "ERROR" "Matching a decorated async property" "") upstream-sweep: 0 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` @@ -243,48 +239,52 @@ Rule shape: ```yaml rules: -- id: test - options: - taint_unify_mvars: true - mode: taint - pattern-sources: - - patterns: - - pattern-inside: function $FUNC(...) { ... } - - pattern: source(...) - pattern-sinks: - - patterns: - - pattern-inside: $FUNC($TAINT) - - pattern: $TAINT - languages: [js] +- id: decorated-async-property + languages: [typescript] + pattern-either: + - pattern: | + @Query(...) + async foo + - pattern: | + @Query(...) + async bar(...) + - pattern: | + @Query(...) + async qux(...){ + ... + } ``` Target: -```javascript -function foo() { - x = source() - foo(x) // expected - bar(x) // current false positive -} +```typescript +class Foo { + @Query() + async bar(){ + } + + @Query() + static async bar(){ + } + + @Query() + async qux(){ + } -function bar() { - x = source() - foo(x) // current false positive - bar(x) // expected + @Query() + static async qux(){ + } } ``` Likely fix area: -- This is now a false-positive issue. Jerboa reports both cross-function calls - (`bar(x)` in `foo`, `foo(x)` in `bar`) instead of requiring `$FUNC` from the - source `pattern-inside` and sink `pattern-inside` to unify. -- The rule option `taint_unify_mvars: true` is parsed but not currently - enforced in taint source/sink compatibility. -- Recommended isolation: - - confirm source findings carry `$FUNC` from `function $FUNC(...) { ... }`; - - confirm sink findings carry `$FUNC` from `$FUNC($TAINT)`; - - require compatible shared metavariables when the option is enabled. +- This is a false-negative TypeScript structural matching gap for decorated + async class methods/properties. +- Existing smoke has `scan TypeScript decorated class fields`; start by + comparing its pattern support with these `@Query()` async method patterns. +- `ts_decorated_static_property` is adjacent and likely shares the same matcher + surface, so verify both after any patch. ## Useful Commands @@ -340,10 +340,10 @@ SCM `function ...` lines are recognized as simple scopes. This avoids sibling leaks without breaking nested callback taint cases already covered by smoke tests. -- After touching taint metavariable unification, run at minimum: +- After touching TypeScript decorated async matching, run at minimum: ```sh make test - SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_unify_mvars|taint_spread_record_big|taint_safe_comparisons|taint_react)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh + SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(ts_decorated_async_property|ts_decorated_static_property|taint_unify_mvars)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle ``` --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -10116,6 +10116,12 @@ (rule-option-enabled? rule "taint_assume_safe_indexes")) (def (taint-assume-safe-functions? rule) (rule-option-enabled? rule "taint_assume_safe_functions")) + (def (taint-unify-mvars? rule) + (rule-option-enabled? rule "taint_unify_mvars")) + (def (taint-mvars-compatible? rule source sink) + (or (not (taint-unify-mvars? rule)) + (and (bindings-compatible? source sink) + (bindings-compatible? sink source)))) (def (taint-only-propagate-through-assignments? rule) (rule-option-enabled? rule @@ -10916,6 +10922,7 @@ (and source sink (same-simple-function-scope? source-text source sink) + (taint-mvars-compatible? rule source sink) (or (and non-exact (or (not (taint-state-token? source-state)) (source-state-before-sink? --- 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" . "781446E52FF37899") - ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/scan.ss" . "6240D2B852D655ED") ("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 @@ -10751,6 +10751,14 @@ (def (taint-assume-safe-functions? rule) (rule-option-enabled? rule "taint_assume_safe_functions")) +(def (taint-unify-mvars? rule) + (rule-option-enabled? rule "taint_unify_mvars")) + +(def (taint-mvars-compatible? rule source sink) + (or (not (taint-unify-mvars? rule)) + (and (bindings-compatible? source sink) + (bindings-compatible? sink source)))) + (def (taint-only-propagate-through-assignments? rule) (rule-option-enabled? rule "taint_only_propagate_through_assignments")) @@ -11480,6 +11488,7 @@ (and source sink (same-simple-function-scope? source-text source sink) + (taint-mvars-compatible? rule source sink) (or (and non-exact (or (not (taint-state-token? source-state)) (source-state-before-sink? source-state source sink) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1921,6 +1921,19 @@ "source(a)\nother(b)\nsink(a)\nsink(b)\n")]) (check (length findings) => 0))) +(test-case "scan JavaScript taint unifies source and sink metavariables" + (let* ([taint-config + "rules:\n - id: demo.taint.unify-mvars\n mode: taint\n languages: [javascript]\n message: unified sink\n severity: WARNING\n options:\n taint_unify_mvars: true\n pattern-sources:\n - patterns:\n - pattern-inside: function $FUNC(...) { ... }\n - pattern: source(...)\n pattern-sinks:\n - patterns:\n - pattern-inside: $FUNC($TAINT)\n - pattern: $TAINT\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function foo() {\n x = source()\n foo(x)\n bar(x)\n}\nfunction bar() {\n x = source()\n foo(x)\n bar(x)\n}\n")]) + (check (length findings) => 2) + (check (finding-start-line (car findings)) => 3) + (check (finding-start-line (cadr findings)) => 9))) + (test-case "scan taint labels keep sibling if else assignments separate" (let* ([taint-config "rules:\n - id: demo.taint.labels.branches\n mode: taint\n languages: [python]\n message: labeled branch sink\n severity: WARNING\n pattern-sources:\n - label: TAINTED\n pattern: source(...)\n - label: CLEANED\n pattern: sanitize(...)\n pattern-sinks:\n - requires: TAINTED and not CLEANED\n pattern: sink(...)\n"]