Respect JavaScript branch assignment taint
ober
395597039b3e48cd0fc42fe015d85834611ae84c
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,26 +1,25 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 08:10 MDT +Date: 2026-05-30 08:39 MDT Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` Branch: `main` Base HEAD before this checkpoint: -`acca729 Cover JavaScript SQL concat taint` +`3213521 Trim JavaScript template metavariable ranges` The user wants the pure Jerboa Semgrep port carried forward until it reaches Semgrep parity. Do not treat this handoff as completion. This checkpoint closes -the remaining upstream JavaScript taint `simpl_nodejs_eval` mismatch by -matching Semgrep's JavaScript template-string metavariable ranges: include the -opening backtick but exclude the closing backtick. +the upstream JavaScript taint `eslint_obj_inj` mismatch by preventing a clean +assignment in a mutually exclusive JavaScript `if`/`else` branch from killing a +tainted assignment made in its sibling branch. ## Immediate State -The worktree was clean at `acca729` before this checkpoint. The implementation -change is in `src/semgrep/scan.ss` and -`src/semgrep/match/structural.ss`; `lib/semgrep/scan.sls`, -`lib/semgrep/match/structural.sls`, and `src/.jerbuild-hashes` were -regenerated by `make test`. +The worktree was clean at `3213521` before this checkpoint. The implementation +change is in `src/semgrep/scan.ss`; `lib/semgrep/scan.sls` and +`src/.jerbuild-hashes` were regenerated by `make test`. A focused smoke test +was added in `tests/smoke.ss`. Current headline: @@ -29,10 +28,10 @@ Current headline: - Promoted JavaScript pattern oracle: 91 passed / 0 mismatched. - Promoted Python pattern oracle: 116 passed / 0 mismatched. - Local oracle: 42 passed / 0 failed. -- Smoke suite: 305 tests / 305 passed. +- Smoke suite: 306 tests / 306 passed. - Broad same-basename upstream `tests/rules` sweep: 437 passed / 0 mismatched / 0 Jerboa errors, with 3 packaged-Semgrep current errors. -- Upstream `tests/tainting_rules/js` sweep: 9 passed / 2 mismatched / +- Upstream `tests/tainting_rules/js` sweep: 10 passed / 1 mismatched / 0 Jerboa errors / 0 current errors. Semgrep parity is not reached. The cleanest active frontier is JavaScript taint @@ -131,7 +130,7 @@ Operational defaults: finding in `simpl_nodejs_eval`. - JS tainting-rule sweep became 8 passed / 3 mismatched. -This checkpoint: +`3213521 Trim JavaScript template metavariable ranges` - Adds `template-finding-end-byte` in `src/semgrep/scan.ss` so structural findings on JavaScript `template_string` nodes end before the closing @@ -145,13 +144,24 @@ This checkpoint: - Closes upstream JS tainting-rule case `simpl_nodejs_eval`. - JS tainting-rule sweep is now 9 passed / 2 mismatched. +This checkpoint: + +- Adds JavaScript-specific mutually exclusive branch detection for assignment + kill logic in `src/semgrep/scan.ss`. +- Keeps the existing Python branch detection but routes taint assignment kills + through a language-agnostic wrapper. +- Prevents an assignment like `c = 1` in an `else` branch from killing taint + introduced by `c = x` in the sibling `if` branch. +- Adds smoke coverage: + `scan JavaScript taint branch assignment survives alternate clean assignment`. +- Closes upstream JS tainting-rule case `eslint_obj_inj`. +- JS tainting-rule sweep is now 10 passed / 1 mismatched. + This checkpoint modifies: ```text HANDOFF_OPUS_4_8.md -src/semgrep/match/structural.ss src/semgrep/scan.ss -lib/semgrep/match/structural.sls lib/semgrep/scan.sls src/.jerbuild-hashes tests/smoke.ss @@ -159,6 +169,113 @@ tests/smoke.ss ## Latest Code Change Details +### JavaScript Mutually Exclusive Branch Assignment Kills + +The upstream `eslint_obj_inj` rule treats function parameters and calls as +taint sources, and object index expressions as sinks: + +```yaml +pattern-sources: + - pattern-either: + - patterns: + - pattern-inside: | + function ...(..., $PARAM, ...) { + ... + } + - pattern: $PARAM + - pattern: $F(...) +pattern-sinks: + - patterns: + - pattern-inside: $OBJ[$SINK] + - pattern: $SINK +``` + +The remaining missed target was: + +```javascript +function test3(x) { + var c + if (z) + c = x + else + c = 1 + return o[c] +} +``` + +Semgrep reports `return o[c]` because there is a feasible branch where `c` +receives tainted `x`. Jerboa previously propagated the source through +`c = x`, then killed that propagated source when it saw the later clean +assignment `c = 1`. That kill was too path-insensitive: assignments in sibling +branches should not kill one another. + +The scanner already had Python indentation-based detection for mutually +exclusive `if` branches. This checkpoint adds a JavaScript counterpart: + +- `javascript-significant-trimmed-line` +- `javascript-strip-leading-close-braces` +- `javascript-if-clause-kind` +- `javascript-parent-if-clause` +- `javascript-if-chain-start` +- `javascript-if-branch-info` +- `javascript-findings-in-mutually-exclusive-if-branches?` +- `findings-in-mutually-exclusive-if-branches?` + +Relevant code locations: + +- `python-findings-in-mutually-exclusive-if-branches?`: around line 30256 in + [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss). +- `javascript-findings-in-mutually-exclusive-if-branches?`: around line 30358. +- `findings-in-mutually-exclusive-if-branches?`: around line 30370. +- `taint-assignment-kills-source?`: around line 31619. + +`taint-assignment-kills-source?` now calls the shared wrapper. The kill is +suppressed only when the source finding and clean assignment finding are in +different branches of the same detected `if`/`else if`/`else` chain. Same-branch +and same-scope kills still apply, so reassignment cases such as `x = clean` +before a sink remain covered by the existing smoke tests. + +The JavaScript helper is deliberately small and line-oriented. It handles the +fixture's unbraced shape: + +```javascript +if (z) + c = x +else + c = 1 +``` + +and also strips leading close braces so lines shaped like `} else {` can be +classified. It is not a complete JavaScript control-flow graph. If future +cases expose braced branch false positives/negatives, prefer tightening this +helper or replacing it with AST-backed branch ownership instead of loosening +taint reachability globally. + +Smoke coverage added: + +```scheme +(test-case "scan JavaScript taint branch assignment survives alternate clean assignment" + ...) +``` + +The smoke includes both the reporting `test3` shape above and a non-reporting +control: + +```javascript +function test4(x) { + var d + if (x) + d = 1 + else + d = 2 + return o[d] +} +``` + +That guard matters because the rule also treats function parameters as sources; +the clean branch-assignment fix must not make every branch-assigned index +tainted. + ### JavaScript Template String Metavariable Ranges Semgrep reports a JavaScript template-string metavariable range from the @@ -321,11 +438,11 @@ The scanner now separates carrier tokens from unification constraints for Relevant locations in [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): -- `source-token-in-sink/except?`: around line 30325. -- `taint-unify-mvars?`: around line 30463. -- `taint-state-mvars-compatible?`: around line 30471. -- `taint-unify-ignored-source-token-names`: around line 30493. -- Taint reachability uses those helpers around lines 31230 and 31279. +- `source-token-in-sink/except?`: around line 30795. +- `taint-unify-mvars?`: around line 30933. +- `taint-state-mvars-compatible?`: around line 30941. +- `taint-unify-ignored-source-token-names`: around line 30963. +- Taint reachability uses those helpers around lines 31700 and 31750. If future taint work touches token reachability, rerun `metavar_eq_simple` before trusting the result. @@ -343,7 +460,22 @@ make test Result: ```text -305 tests, 305 passed, 0 failed +306 tests, 306 passed, 0 failed +``` + +Focused upstream JS taint `eslint_obj_inj`: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(eslint_obj_inj)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ +tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` Focused upstream JS taint `metavar_eq_simple`: @@ -393,7 +525,7 @@ Broad same-basename upstream `tests/rules` sweep: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ -LIST_MISMATCHES=1 MAX_DIFFS=40 tests/oracle/upstream-sweep.sh +LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh ``` Result: @@ -456,34 +588,37 @@ Full JS tainting-rule subdirectory: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ -LIST_MISMATCHES=1 MAX_DIFFS=80 tests/oracle/upstream-sweep.sh +LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 9 passed, 2 mismatched, 0 jerboa errors, 0 current errors, 11 compared +upstream-sweep: 10 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 11 compared ``` -## Remaining JS Taint Mismatches - -The 2 remaining mismatches under `/Users/user/mine/semgrep/tests/tainting_rules/js` -are: +## Remaining JS Taint Mismatch -1. `eslint_obj_inj` +The 1 remaining mismatch under `/Users/user/mine/semgrep/tests/tainting_rules/js` +is: -Semgrep reports `return o[c]` after `c` is conditionally assigned from tainted -parameter `x`; Jerboa misses the branch-merged taint. Likely gap: -path-insensitive propagation from conditional assignments such as -`if (...) c = x else c = 1`. - -2. `metavar_eq_conditional` +1. `metavar_eq_conditional` Semgrep keeps both possible taint bindings from `cond ? get(A) : get(B)` and reports both `A` and `B` messages at both sinks. Jerboa keeps only the locally matching binding at each sink. Likely gap: conditional-expression sources need multiple alternative taint states/metavariable environments. +Current diff shape: + +```diff +@@ -1,4 +1,2 @@ +-(finding "test-metavar-eq" ".../metavar_eq_conditional.js" 12 1 209 12 15 223 "WARNING" "Matched on A!" "") + (finding "test-metavar-eq" ".../metavar_eq_conditional.js" 12 1 209 12 15 223 "WARNING" "Matched on B!" "") + (finding "test-metavar-eq" ".../metavar_eq_conditional.js" 9 1 167 9 15 181 "WARNING" "Matched on A!" "") +-(finding "test-metavar-eq" ".../metavar_eq_conditional.js" 9 1 167 9 15 181 "WARNING" "Matched on B!" "") +``` + ## Completed Target: `sanitized_by_side_effect` Verification command: @@ -818,22 +953,104 @@ Actual improvement: - Broad `tests/rules`: still 0 mismatches and 0 Jerboa errors, with the same 3 current-side packaged-Semgrep errors. +## Completed Target: `eslint_obj_inj` + +Verification command: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(eslint_obj_inj)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ +tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +Old diff before this checkpoint: + +```diff +@@ -1,3 +1,2 @@ + (finding "tainting" ".../eslint_obj_inj.js" 10 15 151 10 16 152 ...) +-(finding "tainting" ".../eslint_obj_inj.js" 21 14 286 21 15 287 ...) + (finding "tainting" ".../eslint_obj_inj.js" 4 14 70 4 15 71 ...) +``` + +Relevant target shape: + +```javascript +function test3(x) { + var c + if (z) + c = x + else + c = 1 + return o[c] +} +``` + +The source from `c = x` was correctly propagated to token `c`, but +`taint-assignment-kills-source?` later saw `c = 1` in the sibling `else` +branch and killed the propagated state before the `return o[c]` sink. The fix +is intentionally narrow: the clean assignment no longer kills a source when the +source finding and assignment finding are in mutually exclusive branches of the +same JavaScript `if` chain. + +Validation run for this patch: + +```sh +make test +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(eslint_obj_inj)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ +tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +LIST_MISMATCHES=1 MAX_DIFFS=240 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ +LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh +git diff --check +``` + +Actual improvement: + +- Focused `eslint_obj_inj`: 1 passed / 0 mismatched. +- Full JS tainting rules: 10 passed / 1 mismatched. +- Smoke: one more test, all passing. +- Local oracle: 42 passed / 0 failed. +- Broad `tests/rules`: still 0 mismatches and 0 Jerboa errors, with the same + 3 current-side packaged-Semgrep errors. + ## Recommended Next Target -The next target is probably `eslint_obj_inj`, because the missing report is a -single taint-flow gap: +The next target is `metavar_eq_conditional`, the last currently known mismatch +in upstream `tests/tainting_rules/js`: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ -CASE_REGEX='^(eslint_obj_inj)$' LIST_MISMATCHES=1 MAX_DIFFS=160 \ +CASE_REGEX='^(metavar_eq_conditional)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ tests/oracle/upstream-sweep.sh ``` -Semgrep reports line 21 `return o[c]` after `c` is conditionally assigned from -tainted parameter `x`; Jerboa currently reports only the simpler flows. The -likely fix is to model branch-merged assignments for JavaScript taint, not to -globally loosen source/sink containment. +Semgrep reports both `Matched on A!` and `Matched on B!` at both sink calls +after: + +```javascript +var source = cond ? get(A) : get(B) +``` + +Jerboa currently reports only the locally matching message for each sink. The +likely fix is to represent conditional-expression sources as multiple +alternative taint states with distinct metavariable environments, then preserve +both through assignment propagation to `source`. Do not solve this by loosening +`taint_unify_mvars` compatibility globally; that would risk regressing +`metavar_eq_simple`, which is already fixed. If touching conditional-source logic, keep `metavar_eq_simple` and `metavar_eq_conditional` separate in your head: --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -30255,6 +30255,133 @@ (alist-ref/default b-branch 'chain-start -2)) (not (= (alist-ref/default a-branch 'start -1) (alist-ref/default b-branch 'start -1)))))) + (def (javascript-significant-trimmed-line source line-start) + (let* ([line-end (line-end-after source line-start)] + [trimmed (string-trim + (substring source line-start line-end))]) + (and (> (string-length trimmed) 0) + (not (sg-string-prefix? "//" trimmed)) + trimmed))) + (def (javascript-strip-leading-close-braces text) + (let ([len (string-length text)]) + (let loop ([i 0]) + (cond + [(>= i len) ""] + [(or (char-whitespace? (string-ref text i)) + (char=? (string-ref text i) #\})) + (loop (+ i 1))] + [else (substring text i len)])))) + (def (javascript-if-clause-kind trimmed) + (let ([normalized (javascript-strip-leading-close-braces + trimmed)]) + (cond + [(or (sg-string-prefix? "if " normalized) + (sg-string-prefix? "if(" normalized)) + 'if] + [(or (sg-string-prefix? "else if " normalized) + (sg-string-prefix? "else if(" normalized)) + 'elif] + [(or (string=? normalized "else") + (sg-string-prefix? "else " normalized) + (sg-string-prefix? "else{" normalized)) + 'else] + [else #f]))) + (def (javascript-parent-if-clause source offset) + (let* ([line-start (line-start-before source offset)] + [target-indent (line-indent-at-offset source offset)]) + (let loop ([current-start line-start] + [current-indent target-indent]) + (let ([prev-start (python-previous-line-start + source + current-start)]) + (if (not prev-start) + #f + (let ([trimmed (javascript-significant-trimmed-line + source + prev-start)]) + (if (not trimmed) + (loop prev-start current-indent) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (if (>= indent current-indent) + (loop prev-start current-indent) + (let ([kind (javascript-if-clause-kind + trimmed)]) + (if kind + (list + (cons 'kind kind) + (cons 'indent indent) + (cons 'start prev-start)) + (loop prev-start indent)))))))))))) + (def (javascript-if-chain-start source clause) + (let ([kind (alist-ref/default clause 'kind #f)] + [clause-start (alist-ref/default clause 'start #f)] + [clause-indent (alist-ref/default clause 'indent 0)]) + (cond + [(eq? kind 'if) clause-start] + [(or (eq? kind 'elif) (eq? kind 'else)) + (let loop ([current-start clause-start]) + (let ([prev-start (python-previous-line-start + source + current-start)]) + (if (not prev-start) + #f + (let ([trimmed (javascript-significant-trimmed-line + source + prev-start)]) + (if (not trimmed) + (loop prev-start) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (cond + [(< indent clause-indent) #f] + [(> indent clause-indent) (loop prev-start)] + [(eq? (javascript-if-clause-kind trimmed) + 'if) + prev-start] + [else (loop prev-start)])))))))] + [else #f]))) + (def (javascript-if-branch-info source finding) + (let ([clause (javascript-parent-if-clause + source + (finding-start-offset finding))]) + (and clause + (let ([chain-start (javascript-if-chain-start + source + clause)]) + (and chain-start + (list + (cons 'start (alist-ref/default clause 'start #f)) + (cons 'indent (alist-ref/default clause 'indent 0)) + (cons 'chain-start chain-start))))))) + (def (javascript-findings-in-mutually-exclusive-if-branches? + source + a + b) + (let ([a-branch (javascript-if-branch-info source a)] + [b-branch (javascript-if-branch-info source b)]) + (and a-branch + b-branch + (= (alist-ref/default a-branch 'indent -1) + (alist-ref/default b-branch 'indent -2)) + (= (alist-ref/default a-branch 'chain-start -1) + (alist-ref/default b-branch 'chain-start -2)) + (not (= (alist-ref/default a-branch 'start -1) + (alist-ref/default b-branch 'start -1)))))) + (def (findings-in-mutually-exclusive-if-branches? + source + a + b) + (or (python-findings-in-mutually-exclusive-if-branches? + source + a + b) + (javascript-findings-in-mutually-exclusive-if-branches? + source + a + b))) (def (python-pass-line? trimmed) (or (string=? trimmed "pass") (sg-string-prefix? "pass #" trimmed))) @@ -31577,7 +31704,7 @@ source-text assignment-finding sink) - (not (python-findings-in-mutually-exclusive-if-branches? + (not (findings-in-mutually-exclusive-if-branches? source-text source assignment-finding)) --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,11 +3,11 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "6982E07679D20836") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "23B1556A1042F11C") - ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/scan.ss" . "C3882074EBC22BFC") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") - ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") + ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("src/semgrep/match/structural.ss" . "6FE77014EE9FDCE4") ("src/semgrep/main.ss" . "A4EC9E7F2A09D25E") ("src/semgrep/cli.ss" . "EBDC4B1DAD3F13CC")) --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -30265,6 +30265,112 @@ (not (= (alist-ref/default a-branch 'start -1) (alist-ref/default b-branch 'start -1)))))) +(def (javascript-significant-trimmed-line source line-start) + (let* ([line-end (line-end-after source line-start)] + [trimmed (string-trim (substring source line-start line-end))]) + (and (> (string-length trimmed) 0) + (not (sg-string-prefix? "//" trimmed)) + trimmed))) + +(def (javascript-strip-leading-close-braces text) + (let ([len (string-length text)]) + (let loop ([i 0]) + (cond + [(>= i len) ""] + [(or (char-whitespace? (string-ref text i)) + (char=? (string-ref text i) #\})) + (loop (+ i 1))] + [else (substring text i len)])))) + +(def (javascript-if-clause-kind trimmed) + (let ([normalized (javascript-strip-leading-close-braces trimmed)]) + (cond + [(or (sg-string-prefix? "if " normalized) + (sg-string-prefix? "if(" normalized)) + 'if] + [(or (sg-string-prefix? "else if " normalized) + (sg-string-prefix? "else if(" normalized)) + 'elif] + [(or (string=? normalized "else") + (sg-string-prefix? "else " normalized) + (sg-string-prefix? "else{" normalized)) + 'else] + [else #f]))) + +(def (javascript-parent-if-clause source offset) + (let* ([line-start (line-start-before source offset)] + [target-indent (line-indent-at-offset source offset)]) + (let loop ([current-start line-start] [current-indent target-indent]) + (let ([prev-start (python-previous-line-start source current-start)]) + (if (not prev-start) + #f + (let ([trimmed + (javascript-significant-trimmed-line source prev-start)]) + (if (not trimmed) + (loop prev-start current-indent) + (let ([indent (line-indent-at-offset source prev-start)]) + (if (>= indent current-indent) + (loop prev-start current-indent) + (let ([kind (javascript-if-clause-kind trimmed)]) + (if kind + (list (cons 'kind kind) + (cons 'indent indent) + (cons 'start prev-start)) + (loop prev-start indent)))))))))))) + +(def (javascript-if-chain-start source clause) + (let ([kind (alist-ref/default clause 'kind #f)] + [clause-start (alist-ref/default clause 'start #f)] + [clause-indent (alist-ref/default clause 'indent 0)]) + (cond + [(eq? kind 'if) clause-start] + [(or (eq? kind 'elif) (eq? kind 'else)) + (let loop ([current-start clause-start]) + (let ([prev-start (python-previous-line-start source current-start)]) + (if (not prev-start) + #f + (let ([trimmed + (javascript-significant-trimmed-line source prev-start)]) + (if (not trimmed) + (loop prev-start) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (cond + [(< indent clause-indent) #f] + [(> indent clause-indent) (loop prev-start)] + [(eq? (javascript-if-clause-kind trimmed) 'if) + prev-start] + [else (loop prev-start)])))))))] + [else #f]))) + +(def (javascript-if-branch-info source finding) + (let ([clause (javascript-parent-if-clause + source + (finding-start-offset finding))]) + (and clause + (let ([chain-start (javascript-if-chain-start source clause)]) + (and chain-start + (list (cons 'start (alist-ref/default clause 'start #f)) + (cons 'indent (alist-ref/default clause 'indent 0)) + (cons 'chain-start chain-start))))))) + +(def (javascript-findings-in-mutually-exclusive-if-branches? source a b) + (let ([a-branch (javascript-if-branch-info source a)] + [b-branch (javascript-if-branch-info source b)]) + (and a-branch + b-branch + (= (alist-ref/default a-branch 'indent -1) + (alist-ref/default b-branch 'indent -2)) + (= (alist-ref/default a-branch 'chain-start -1) + (alist-ref/default b-branch 'chain-start -2)) + (not (= (alist-ref/default a-branch 'start -1) + (alist-ref/default b-branch 'start -1)))))) + +(def (findings-in-mutually-exclusive-if-branches? source a b) + (or (python-findings-in-mutually-exclusive-if-branches? source a b) + (javascript-findings-in-mutually-exclusive-if-branches? source a b))) + (def (python-pass-line? trimmed) (or (string=? trimmed "pass") (sg-string-prefix? "pass #" trimmed))) @@ -31523,7 +31629,7 @@ to-binding (same-simple-function-scope? source-text source assignment-finding) (same-simple-function-scope? source-text assignment-finding sink) - (not (python-findings-in-mutually-exclusive-if-branches? + (not (findings-in-mutually-exclusive-if-branches? source-text source assignment-finding)) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -4104,6 +4104,18 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 6))) +(test-case "scan JavaScript taint branch assignment survives alternate clean assignment" + (let* ([taint-config + "rules:\n - id: demo.taint.js.branch.assignment\n mode: taint\n languages: [javascript]\n message: branch assignment\n severity: WARNING\n pattern-sources:\n - pattern-either:\n - patterns:\n - pattern-inside: |\n function ...(..., $PARAM, ...) {\n ...\n }\n - pattern: $PARAM\n - pattern: $F(...)\n pattern-sinks:\n - patterns:\n - pattern-inside: $OBJ[$SINK]\n - pattern: $SINK\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function test3(x) {\n var c\n if (z)\n c = x\n else\n c = 1\n return o[c]\n}\n\nfunction test4(x) {\n var d\n if (x)\n d = 1\n else\n d = 2\n return o[d]\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 7))) + (test-case "scan JavaScript taint source under either pattern-inside" (let* ([taint-config "rules:\n - id: demo.taint.js.inside-either-source\n mode: taint\n languages: [javascript]\n message: inside either source\n severity: WARNING\n pattern-sources:\n - patterns:\n - pattern-either:\n - pattern-inside: function ... ($REQ, $RES) {...}\n - pattern-inside: function ... ($REQ, $RES, $NEXT) {...}\n - patterns:\n - pattern: $REQ.on('data', function ($CHUNK) { ... })\n - focus-metavariable: $CHUNK\n pattern-sinks:\n - pattern: sink(...)\n"]