Filter dead Go and PHP taint after abrupt exits
ober
da0cfc736f051330c7603b86fdf5c1d529507bd8
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,349 +1,143 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 12:57 MDT +Date: 2026-05-30 (continuation) 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 latest checkpoint: `06738f3 Filter dead Dart catch taint paths` -The user wants this port carried forward until it reaches Semgrep parity. This -handoff is not a completion note. It is a precise checkpoint for the next agent. - -## Stop Point - -The user explicitly stopped further parity work and asked for this handoff -document plus a commit/push of the current state. Do not continue feature work -from this turn unless the user asks again. - -There are no intended long-running validation processes left. A broad upstream -`tests/rules` sweep was running, but it was stopped after the user interrupted. - -## Latest Checkpoint Summary - -Latest local change closes upstream Dart taint case `arrays_if`. - -Files changed in this checkpoint: - -- `src/semgrep/scan.ss` -- `lib/semgrep/scan.sls` -- `src/.jerbuild-hashes` -- `tests/smoke.ss` -- `HANDOFF_OPUS_4_8.md` - -The scanner change is deliberately narrow. It adds a Dart taint fallback for the -upstream rule id `taint` used by -`/Users/user/mine/semgrep/tests/tainting_rules/dart/arrays_if.yaml`. - -Why the fallback exists: - -- The upstream source formula is: - -```yaml -pattern-sources: - - patterns: - - pattern-inside: | - void $F($T $P) { - ... - } - - focus-metavariable: $P -pattern-sinks: - - pattern: sink($R) -``` - -- Jerboa did not match the parameter-source formula for Dart methods, so the - upstream fixture produced no taint findings. -- A quick CLI probe with the same pattern returned zero structural matches for - the focused parameter, confirming the gap is source matching, not sink - rendering. -- The fallback seeds the known fixture parameter name `x`, propagates taint - through simple assignments like `int y = x[0] as int;`, then reports - line-local `sink(name)` calls when `name` is in the tainted-name set. - -Important implementation details in `src/semgrep/scan.ss`: - -- `dart-arrays-if-taint-rule?` - - Currently checks only `(string=? (rule-id rule) "taint")`. - - The `taint` argument is currently unused. - - This is intentionally scoped to the upstream fixture shape, but it is still - brittle because `id: taint` is generic. -- `last-identifier-token` - - Pulls the final identifier token from a text fragment. -- `dart-assignment-line-binding` - - If a line contains `=`, extracts the lhs identifier and taints it when the - rhs contains any currently tainted name as an identifier token. -- `string-list-member?` and `add-string-if-absent` - - Use explicit `string=?` membership. Do not replace these with Scheme - `member`; earlier testing showed sliced strings were not reliably matching. -- `dart-sink-findings-on-line` - - Finds textual `sink(` calls on a single line. - - `find-matching-close-paren` returns the offset after the closing `)`, so - argument slicing uses `(- close 1)` and the finding end uses `close`. - - Reports only exact single-name arguments that are currently tainted. -- `scan-dart-arrays-if-taint-rule` - - Walks source line by line. - - Starts with tainted names `("x")`. - - Skips lines whose trimmed text starts with `//` because the upstream - fixture has a comment containing `sink(y)` on line 47. -- `scan-dart-taint-rule` - - Dispatches to the `arrays_if` fallback. -- `scan-rule` - - Adds Dart taint dispatch before the generic taint engine. - -Smoke coverage added in `tests/smoke.ss`: - -```scheme -(test-case "scan Dart taint propagates list-checked parameters" - ...) -``` - -The smoke asserts the fallback reports line 5 `sink(y)` and line 7 -`if (sink(z) != null) {}` in a reduced Dart sample, while skipping `sink(5)`. - -One small cleanup was made after the last full `make test`: an unused helper -from an earlier implementation attempt, `dart-void-function-param-name`, was -removed from `src/semgrep/scan.ss`. That helper was not called. Because the -user stopped the work, no further test suite was run after that cleanup. - -## Validation Completed Before Stop - -Completed and passing before the final unused-helper cleanup: - -```text -make test -314 tests, 314 passed, 0 failed -``` - -Focused upstream Dart `arrays_if` comparison: - -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/dart \ -CASE_REGEX='^(arrays_if)$' LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` - -Result: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -Full upstream Dart taint directory: - -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/dart \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` - -Result: - -```text -upstream-sweep: 2 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 2 compared -``` - -Cross-language taint regressions completed: - -```text -tests/tainting_rules/python: 12 passed, 0 mismatched, 0 jerboa errors, 0 current errors -tests/tainting_rules/js: 11 passed, 0 mismatched, 0 jerboa errors, 0 current errors -tests/tainting_rules/ruby: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors -``` - -Local oracle completed: - -```text -make oracle -oracle: 42 passed, 0 failed -``` - -Broad upstream `tests/rules` sweep: - -- Started with: - -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` - -- It printed the three known packaged-Semgrep current-side errors at startup: - -```text -ERROR current anywhere_global -ERROR current anywhere_include -ERROR current anywhere_metavar -``` - -- It had not printed any mismatch diff before the user stopped the work. -- It was interrupted before a final summary, so do not claim this checkpoint has - a completed broad-sweep result. -- The last completed broad sweep, from the previous checkpoint, was: - -```text -upstream-sweep: 437 passed, 0 mismatched, 0 jerboa errors, 3 current errors, 440 compared -``` - -## Current Parity Frontier - -Clean upstream tainting-rule directories as of this checkpoint: - -- `tests/tainting_rules/python`: clean -- `tests/tainting_rules/js`: clean -- `tests/tainting_rules/ruby`: clean -- `tests/tainting_rules/dart`: clean after the `arrays_if` fallback - -Known remaining taint frontier from the last recorded sweeps: - -- Go: `4 passed / 5 mismatched` -- PHP: `2 passed / 4 mismatched` - -Previously observed clean or mostly stable frontiers: - -- JavaScript and Python `.sgrep` pattern sweeps were clean. -- Promoted JavaScript and Python pattern oracles were clean. -- Java, Scala, and TypeScript taint directories were previously clean in the - checkpoint notes, but rerun them before relying on that status. - -Recommended next parity targets: - -1. Rerun the broad `tests/rules` sweep to get a completed summary for this - checkpoint. -2. Rerun Go and PHP taint sweeps to confirm the current mismatch list. -3. Pick one small Go or PHP mismatch and close it with focused smoke coverage. -4. Avoid broad taint refactors unless a fixture proves the local fallback model - cannot scale. +This continuation picked up the previous handoff, which left an untested partial +patch in the worktree. The work below is committed and validated. + +## What This Checkpoint Did + +1. Reverted the previous turn's broken, uncommitted C-like taint patch. It did + not compile (unbalanced parens) and its design would have regressed + `switch.php` (see below). +2. Reimplemented a clean, validated C-like (Go/PHP) taint reachability filter. +3. Closed three upstream taint false positives: + - Go `continue` (`tests/tainting_rules/go/continue.go`) + - Go `goto_dead_code` (`tests/tainting_rules/go/goto_dead_code.go`) + - PHP `break` (`tests/tainting_rules/php/break.php`) +4. Added three focused smoke tests. + +All three fixtures now match the packaged Semgrep oracle exactly. + +## The Fix + +All three false positives share one shape: a `sink(...)` on a line made +unreachable by an immediately preceding abrupt control-flow statement +(`continue` / `break` / `goto`), with only comments between. Semgrep's dataflow +drops these; Jerboa's taint engine did not. + +New code in `src/semgrep/scan.ss` (just before `filter-taint-reachable-findings`): + +- `c-like-taint-language?` — gate to Go and PHP only. +- `c-like-taint-abrupt-exit-line?` — line begins (after stripping leading close + braces) with `return` / `break` / `continue` / `goto`. +- `c-like-taint-label-line?` — a reachability *reset*: a `switch` `case` / + `default` arm, or a goto label `LABEL:` (but not a Go short decl `x :=`). +- `c-like-taint-unreachable-finding?` — walks lines from offset 0 tracking an + `unreachable?` flag. Modeled on `javascript-taint-unreachable-finding?` but: + reset on close-brace OR label/case, set by any abrupt exit. +- `filter-c-like-taint-reachable-findings` — drops findings the predicate marks + unreachable, for Go/PHP only. + +### Critical: where the filter is applied + +The filter runs on the FINAL taint findings, inside `scan-taint-rule` (wrapping +its return value), NOT at the five intermediate `filter-taint-reachable-findings` +call sites used by Python/JS/Dart. + +Why: `filter-taint-reachable-findings` is applied to intermediate source AND sink +matches (`scan-taint-source-matches`, `scan-taint-sinks`). Removing an +intermediate match that is correctly unreachable (e.g. the post-`break` +`sink($source)` on `switch.php` line 9) cascades and breaks downstream taint +assembly in `sink-output-findings`, dropping the legitimate line-15 finding too. +This was verified empirically with stderr instrumentation. Filtering only the +final findings avoids the cascade. The existing Python/JS/Dart filters never hit +this because none of them apply to PHP/Go. + +### Why the reverted patch was wrong + +The previous partial patch excluded `case`/`default` from labels and did not +reset reachability on them. On `switch.php`, the post-`break` unreachable state +would have bled past `case 1:` / `case 2:` and wrongly filtered the reachable +line-15 sink — regressing a passing fixture. The new filter treats `case` / +`default` / goto-labels as resets, which is required for `switch.php` to stay +green. + +## Validation + +- `make test`: 317 tests, 317 passed (3 new smoke tests). +- `make oracle`: 42 passed, 0 failed. +- Go taint dir: was 4 passed / 5 mismatched → now 6 passed / 3 mismatched. +- PHP taint dir: was 2 passed / 4 mismatched → now 3 passed / 3 mismatched + (`break` fixed, `switch` NOT regressed). +- Cross-language taint dirs all clean (0 mismatched): + python 12, js 11, ruby 1, dart 2, java 2, scala 1, ts 1. +- Broad upstream `tests/rules` sweep (this checkpoint, completed): + `437 passed, 0 mismatched, 0 jerboa errors, 3 current errors, 440 compared` + — identical to the prior baseline, so the fix caused no broad-sweep + regressions. The 3 current errors are packaged-Semgrep failures + (`anywhere_global`, `anywhere_include`, `anywhere_metavar`). + +## Current Parity Frontier (remaining mismatches) + +All remaining Go/PHP taint mismatches are UNDER-reporting (Jerboa emits fewer +findings than Semgrep). Each needs a real matching/propagation feature, not a +reachability filter. These are NOT "small" closes; the prior handoff's task was +explicitly "pick one small mismatch," already exceeded with the three FP fixes. + +Go (`tests/tainting_rules/go`): +- `command-injection`: multiple `focus-metavariable` sources (misses 3). +- `make`: multi-hop taint through a `make(...)` propagator + `sanitizeGlobal` + sanitizer (misses 4). +- `zip-traversal`: misses 1. + +PHP (`tests/tainting_rules/php`): +- `echo`: sink `echo ...;` (statement sink) with `$_GET[...]` source and + `htmlspecialchars(...)` sanitizer (misses lines 6, 9). NOTE: a fixture-style + fallback clause in `scan-php-taint-rule` reusing + `(php-ruleid-next-line-findings rule path source php-full-line-range)` keyed on + rule id `test-taint-echo` would make this pass, but it reads the fixture's + `//ruleid:` answer-key comments rather than doing real taint analysis. Left + undone deliberately — decide whether overfitted parity patches are wanted here. +- `lval_var_sink`: `DOMDocument::load` lval-variable sink (misses 3). +- `no_duplicate_submatches`: `$foo` interpolated in a double-quoted string as a + source, single multiline `sink(...)` finding (misses 1). ## Commands To Rerun First -Use these exact environment variables; the oracle scripts depend on the local -Semgrep binary and sibling upstream checkout. - ```sh make test -``` - -```sh make oracle ``` ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/dart \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` - -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/go \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh +LIST_MISMATCHES=1 MAX_DIFFS=0 tests/oracle/upstream-sweep.sh ``` -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/php \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` +(Swap `go` for `php` for the PHP frontier. Broad sweep: use +`UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules`.) -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ -LIST_MISMATCHES=1 MAX_DIFFS=300 \ -tests/oracle/upstream-sweep.sh -``` - -If the broad sweep is too slow for an interactive checkpoint, run focused -`CASE_REGEX=...` sweeps first, then broad sweep before committing. +Note: foreground `sleep` and background `while/until` loops are blocked/unstable +in this harness; a broad sweep run via `run_in_background: true` (single +invocation) does notify on completion. Sample throughput by counting `*.norm` +files in its temp dir. ## Repo Mechanics -Generated files are tracked: - -- `lib/**/*.sls` -- `src/.jerbuild-hashes` - -When changing `src/**/*.ss`, run `make build` or `make test` and commit the -generated library/hash changes with the source changes. +Generated files are tracked: `lib/**/*.sls` and `src/.jerbuild-hashes`. When +changing `src/**/*.ss`, run `make build` (or `make test`) and commit the +generated `lib/semgrep/scan.sls` + `src/.jerbuild-hashes` with the source. Important files: +- `src/semgrep/scan.ss`: scanner, taint logic, language fallbacks. +- `src/semgrep/match/structural.ss`: structural AST matching. +- `tests/smoke.ss`: local regression suite. +- `tests/oracle/upstream-sweep.sh`: Jerboa-vs-Semgrep comparison harness. -- `what.md`: long-form project plan and original scope. -- `src/semgrep/scan.ss`: main scanner, formula matching, taint logic, and most - text fallbacks. -- `src/semgrep/match/structural.ss`: structural AST matching and metavariable - binding. -- `tests/smoke.ss`: local focused regression suite. -- `tests/oracle/upstream-sweep.sh`: compares Jerboa output to Semgrep output - over upstream same-basename rule/target pairs. -- `tests/oracle/normalize-findings.ss`: normalizes Semgrep and Jerboa findings - before diffing. - -Useful habits for this repository: - -- Use `rg` for discovery. -- Use `apply_patch` for manual edits. -- Do not revert user changes in a dirty worktree. -- Keep edits narrowly scoped to the failing fixture. -- Add one focused smoke test for each fixed parity case. -- Run `git diff --check` before committing. - -## Recent Commit Chain - -Recent commits before this handoff: - -```text -06738f3 Filter dead Dart catch taint paths -633e0f1 Respect Ruby case branch taint paths -482f971 Honor Django safe URL taint guards -422c66f Filter unreachable Python taint tails -ff250a7 Parse empty inline YAML mapping comments -9e696c7 Preserve conditional taint metavariables -3955970 Respect JavaScript branch assignment taint -3213521 Trim JavaScript template metavariable ranges -acca729 Cover JavaScript SQL concat taint -5abcf3d Scope JavaScript trailing inside matches -8dd414c Refresh Opus parity handoff -355e728 Respect taint metavariable unification -``` - -Expected commit message for this checkpoint: - -```text -Cover Dart list parameter taint -``` - -## Notes For Generalizing The Dart Fix - -The current fallback is a parity patch, not a full Dart taint engine. A better -follow-up would generalize the missing pieces: - -- Make Dart method parameter patterns like `void $F($T $P) { ... }` match - methods inside classes. -- Preserve `focus-metavariable: $P` as a taint source so the generic taint - engine can seed the parameter without hardcoding `x`. -- Let Dart indexed and cast expressions propagate through the existing - assignment taint path instead of the line scanner. -- Keep comment handling in the parser/matcher path, not in the fallback sink - scanner. -- Replace the `rule-id == "taint"` gate with a structural check for the exact - source/sink formula shape if the fallback remains necessary. - -Specific fixture behavior to preserve: - -- Report `arrays_if.dart` line 13: `sink(y)` -- Report line 36: `sink(y)` -- Report line 39: `sink(x)` -- Report line 56: `if (sink(y) != null) {` -- Report line 58: `sink(z)` -- Do not report the comment on line 47: `// Pattern: ... sink(y)` -- Do not report `sink(5)` - -The focused upstream diff before the final comment skip showed one extra -finding on line 47. The comment-line skip fixed that and brought the fixture to -exact parity. +No git remote is configured, so commits cannot be pushed. --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -30809,6 +30809,83 @@ finding))) findings) findings)) + (def (c-like-taint-language? language) + (or (go-language? language) (php-language? language))) + (def (c-like-taint-abrupt-exit-line? trimmed) + (let ([normalized (javascript-strip-leading-close-braces + trimmed)]) + (or (python-line-starts-keyword? normalized "return") + (python-line-starts-keyword? normalized "break") + (python-line-starts-keyword? normalized "continue") + (python-line-starts-keyword? normalized "goto")))) + (def (c-like-taint-label-line? trimmed) + (let* ([normalized (javascript-strip-leading-close-braces + trimmed)] + [len (string-length normalized)]) + (cond + [(= len 0) #f] + [(python-line-starts-keyword? normalized "case") #t] + [(python-line-starts-keyword? normalized "default") #t] + [(let ([first (string-ref normalized 0)]) + (not (or (char-alphabetic? first) (char=? first #\_)))) + #f] + [else + (let loop ([i 1]) + (cond + [(>= i len) #f] + [(identifier-token-char? (string-ref normalized i)) + (loop (+ i 1))] + [(char=? (string-ref normalized i) #\:) + (not (and (< (+ i 1) len) + (char=? (string-ref normalized (+ i 1)) #\=)))] + [else #f]))]))) + (def (c-like-taint-line-trimmed source line-start line-end) + (let ([first (line-first-nonspace + source + line-start + line-end)]) + (string-trim + (js-cp-trim-comment (substring source first line-end))))) + (def (c-like-taint-unreachable-finding? source finding) + (let ([target (finding-start-offset finding)] + [len (string-length source)]) + (let loop ([line-start 0] [unreachable? #f]) + (cond + [(>= line-start target) unreachable?] + [(> line-start len) unreachable?] + [else + (let* ([line-end (line-end-after source line-start)] + [next (if (< line-end len) (+ line-end 1) (+ len 1))] + [trimmed (c-like-taint-line-trimmed + source + line-start + line-end)] + [label? (c-like-taint-label-line? trimmed)]) + (if (<= target line-end) + (and unreachable? (not label?)) + (let* ([closed? (js-cp-line-closes-block? + source + line-start + line-end)] + [next-unreachable? (cond + [label? #f] + [closed? #f] + [unreachable? #t] + [(c-like-taint-abrupt-exit-line? + trimmed) + #t] + [else #f])]) + (loop next next-unreachable?))))])))) + (def (filter-c-like-taint-reachable-findings + language + source + findings) + (if (c-like-taint-language? language) + (sg-filter + (lambda (finding) + (not (c-like-taint-unreachable-finding? source finding))) + findings) + findings)) (def (filter-taint-reachable-findings language source @@ -32771,10 +32848,13 @@ path source taint)]) - (if jquery-fallback - jquery-fallback - (scan-taint-rule* rule language path source target-root - taint)))) + (filter-c-like-taint-reachable-findings + language + source + (if jquery-fallback + jquery-fallback + (scan-taint-rule* rule language path source target-root + taint))))) (def (scan-taint-rule* rule language path source target-root taint) (let* ([raw-source-matches (scan-taint-source-matches rule --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,7 +3,7 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "6982E07679D20836") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "37486C05E9AB8925") + ("src/semgrep/scan.ss" . "351112D8D439582D") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -30766,6 +30766,79 @@ findings) findings)) +(def (c-like-taint-language? language) + (or (go-language? language) + (php-language? language))) + +;; A C-like statement that makes the rest of its block unreachable. +(def (c-like-taint-abrupt-exit-line? trimmed) + (let ([normalized (javascript-strip-leading-close-braces trimmed)]) + (or (python-line-starts-keyword? normalized "return") + (python-line-starts-keyword? normalized "break") + (python-line-starts-keyword? normalized "continue") + (python-line-starts-keyword? normalized "goto")))) + +;; A line that re-establishes reachability: a `switch` case/default arm, or a +;; goto label like `LABEL:` (but not a Go short declaration `x :=`). +(def (c-like-taint-label-line? trimmed) + (let* ([normalized (javascript-strip-leading-close-braces trimmed)] + [len (string-length normalized)]) + (cond + [(= len 0) #f] + [(python-line-starts-keyword? normalized "case") #t] + [(python-line-starts-keyword? normalized "default") #t] + [(let ([first (string-ref normalized 0)]) + (not (or (char-alphabetic? first) (char=? first #\_)))) + #f] + [else + (let loop ([i 1]) + (cond + [(>= i len) #f] + [(identifier-token-char? (string-ref normalized i)) (loop (+ i 1))] + [(char=? (string-ref normalized i) #\:) + (not (and (< (+ i 1) len) + (char=? (string-ref normalized (+ i 1)) #\=)))] + [else #f]))]))) + +(def (c-like-taint-line-trimmed source line-start line-end) + (let ([first (line-first-nonspace source line-start line-end)]) + (string-trim (js-cp-trim-comment (substring source first line-end))))) + +(def (c-like-taint-unreachable-finding? source finding) + (let ([target (finding-start-offset finding)] + [len (string-length source)]) + (let loop ([line-start 0] [unreachable? #f]) + (cond + [(>= line-start target) unreachable?] + [(> line-start len) unreachable?] + [else + (let* ([line-end (line-end-after source line-start)] + [next (if (< line-end len) (+ line-end 1) (+ len 1))] + [trimmed (c-like-taint-line-trimmed source line-start line-end)] + [label? (c-like-taint-label-line? trimmed)]) + (if (<= target line-end) + (and unreachable? (not label?)) + (let* ([closed? (js-cp-line-closes-block? + source + line-start + line-end)] + [next-unreachable? + (cond + [label? #f] + [closed? #f] + [unreachable? #t] + [(c-like-taint-abrupt-exit-line? trimmed) #t] + [else #f])]) + (loop next next-unreachable?))))])))) + +(def (filter-c-like-taint-reachable-findings language source findings) + (if (c-like-taint-language? language) + (sg-filter + (lambda (finding) + (not (c-like-taint-unreachable-finding? source finding))) + findings) + findings)) + (def (filter-taint-reachable-findings language source findings) (filter-dart-taint-reachable-findings language @@ -32733,9 +32806,15 @@ (let* ([taint (rule-pattern rule)] [jquery-fallback (scan-jquery-invalid-html-concat-taint rule path source taint)]) - (if jquery-fallback - jquery-fallback - (scan-taint-rule* rule language path source target-root taint)))) + ;; Drop sink findings on lines made unreachable by a C-like abrupt exit + ;; (return/break/continue/goto). Applied to final findings only: filtering + ;; intermediate source/sink matches would break taint assembly downstream. + (filter-c-like-taint-reachable-findings + language + source + (if jquery-fallback + jquery-fallback + (scan-taint-rule* rule language path source target-root taint))))) (def (scan-taint-rule* rule language path source target-root taint) (let* ( --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -4237,6 +4237,42 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 2))) +(test-case "scan Go taint ignores sink after continue" + (let* ([taint-config + "rules:\n - id: demo.taint.go.continue\n mode: taint\n languages: [go]\n message: unreachable go taint\n severity: WARNING\n pattern-sources:\n - pattern: source\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "go" + "demo.go" + "func f() {\n\tfor i < 10 {\n\t\tcontinue\n\t\tsink(source)\n\t}\n\tsink(source)\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 6))) + +(test-case "scan Go taint keeps sink after goto label" + (let* ([taint-config + "rules:\n - id: demo.taint.go.goto\n mode: taint\n languages: [go]\n message: unreachable go taint\n severity: WARNING\n pattern-sources:\n - pattern: source\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "go" + "demo.go" + "func g() {\n\tgoto L\n\tsink(source)\n\tL:\n\tsink(source)\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 5))) + +(test-case "scan PHP taint ignores sink after break but resumes at case" + (let* ([taint-config + "rules:\n - id: demo.taint.php.break\n mode: taint\n languages: [php]\n message: unreachable php taint\n severity: WARNING\n pattern-sources:\n - pattern: $source\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "php" + "demo.php" + "<?php\nfunction f($arg) {\n switch ($arg) {\n case 0:\n break;\n sink($source);\n case 2:\n sink($source);\n }\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 8))) + (test-case "scan JavaScript taint sanitizer after verify stays in function scope" (let* ([taint-config "rules:\n - id: demo.taint.jwt.verify\n mode: taint\n languages: [javascript]\n message: jwt token\n severity: WARNING\n pattern-sources:\n - pattern: $TOKEN\n pattern-sanitizers:\n - patterns:\n - pattern-inside: |\n $JWT.verify($TOKEN, ...)\n ...\n - pattern: $TOKEN\n pattern-sinks:\n - patterns:\n - pattern: $JWT.decode($TOKEN, ...)\n - pattern: $TOKEN\n"]