Cover Dart list parameter taint
ober
e8ac4fdf3cf5466c61bf906407c3e8fd9f30c8d9
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,1237 +1,125 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 12:21 MDT +Date: 2026-05-30 12:57 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: -`633e0f1 Respect Ruby case branch taint paths` - -The user wants the pure Jerboa Semgrep port carried forward until it reaches -Semgrep parity. Do not treat this handoff as completion. The latest checkpoint -closes upstream Dart tainting-rule mismatch `try_return` by filtering -impossible Dart `catch` sources after non-throwing `try` returns and by adding -simple brace-style function scope detection for taint reachability. - -## Immediate State - -The worktree was clean at `633e0f1` before this checkpoint. The latest -implementation change is in -[src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss); -[lib/semgrep/scan.sls](/Users/user/mine/jerboa-semgrep/lib/semgrep/scan.sls) -and [src/.jerbuild-hashes](/Users/user/mine/jerboa-semgrep/src/.jerbuild-hashes) -were regenerated by `make test`. Focused Dart `try`/`catch` taint smoke -coverage was added in [tests/smoke.ss](/Users/user/mine/jerboa-semgrep/tests/smoke.ss). - -Current headline: - -- Full Python `.sgrep` pattern sweep: 164 passed / 0 mismatched. -- Full JavaScript `.sgrep` pattern sweep: 132 passed / 0 mismatched. -- Promoted JavaScript pattern oracle: 91 passed / 0 mismatched. -- Promoted Python pattern oracle: 116 passed / 0 mismatched. -- Local oracle: 42 passed / 0 failed. -- Smoke suite: 313 tests / 313 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: 11 passed / 0 mismatched / - 0 Jerboa errors / 0 current errors. -- Upstream `tests/tainting_rules/python` sweep: 12 passed / 0 mismatched / - 0 Jerboa errors / 0 current errors. -- Upstream `tests/tainting_rules/ruby` sweep: 1 passed / 0 mismatched / - 0 Jerboa errors / 0 current errors. -- Upstream `tests/tainting_rules/dart` sweep: 1 passed / 1 mismatched / - 0 Jerboa errors / 0 current errors. Remaining mismatch: `arrays_if`. - -Semgrep parity is not reached. The JavaScript, Python, and Ruby -`tests/tainting_rules` subdirectories are clean against the packaged oracle. -The active frontier is now semantic taint-control-flow parity in the remaining -upstream `tests/tainting_rules` subdirectories, especially Dart `arrays_if`, -then Go and PHP. - -## Repository Context - -The long-form port plan is [what.md](/Users/user/mine/jerboa-semgrep/what.md). -It is still useful for architecture and scope, but some status counts there are -older than this handoff. Prefer the numbers in this handoff for current -baselines. - -Important local implementation files: - -- [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss) - is the main matcher, formula, and taint implementation. -- [src/semgrep/match/structural.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/match/structural.ss) - owns structural AST matching and metavariable bindings. -- [lib/semgrep/scan.sls](/Users/user/mine/jerboa-semgrep/lib/semgrep/scan.sls) - is generated from `src/semgrep/scan.ss` and is tracked. -- [tests/smoke.ss](/Users/user/mine/jerboa-semgrep/tests/smoke.ss) is the - local smoke suite. -- [tests/oracle/upstream-sweep.sh](/Users/user/mine/jerboa-semgrep/tests/oracle/upstream-sweep.sh) - compares Jerboa against the current Semgrep oracle on upstream same-basename - rule/target pairs. -- [tests/oracle/normalize-findings.ss](/Users/user/mine/jerboa-semgrep/tests/oracle/normalize-findings.ss) - normalizes both outputs for parity comparisons. - -Generated/tracked files: - -- `lib/**/*.sls` and `src/.jerbuild-hashes` are generated by `make build` or - `make test`. -- If `src/**/*.ss` changes, expect the matching `lib/**/*.sls` files and - `src/.jerbuild-hashes` to change too. Commit them together. - -Operational defaults: - -- Use `rg` for code discovery. -- Use `apply_patch` for manual edits. -- Do not revert unrelated user changes in a dirty worktree. -- Keep `SEMGREP_CURRENT=/Users/user/.local/bin/semgrep` in oracle commands. -- Use the sibling checkout under `/Users/user/mine/semgrep` as the upstream - fixture source. - -## Recent Checkpoint Chain - -`4e0eedf Cover JavaScript deep pattern fixtures` - -- Closed the remaining JavaScript `.sgrep` pattern fixture mismatches. -- Full JS pattern sweep became 132 passed / 0 mismatched. - -`1d6987f Cover JavaScript taint throw reachability` - -- Started the upstream JS taint frontier. -- Normalized `tests/tainting_rules` check-id prefixes in the oracle normalizer. -- Fixed unreachable sinks after `throw`. -- JS tainting-rule sweep became 5 passed / 6 mismatched. - -`355e728 Respect taint metavariable unification` - -- Fixed propagated-source handling for - `options: taint_unify_mvars: true`. -- JS tainting-rule sweep became 6 passed / 5 mismatched. -- Added smoke coverage: - `scan JavaScript taint unifies propagated source metavariables`. - -`8dd414c Refresh Opus parity handoff` - -- Handoff-only checkpoint after the `taint_unify_mvars` fix. -- Preserved the investigation notes for `sanitized_by_side_effect`. - -`5abcf3d Scope JavaScript trailing inside matches` - -- Adds `javascript-matching-close-brace` and `javascript-enclosing-block-end` - in `src/semgrep/scan.ss`. -- Makes `inside-after-clause-apply` require after-clause candidates to end - inside the base match's enclosing JavaScript block. -- Adds smoke coverage: - `scan JavaScript taint sanitizer after verify stays in function scope`. -- Closes upstream JS tainting-rule case `sanitized_by_side_effect`. -- JS tainting-rule sweep is now 7 passed / 4 mismatched. - -`acca729 Cover JavaScript SQL concat taint` - -- Adds a JavaScript fallback for quoted-string concat patterns shaped like - `"$SQLSTR" + $EXPR`, preserving `$SQLSTR` and `$EXPR` bindings. -- Treats SQL concat matches as ending at the first non-literal operand after - literal prefixes, matching the upstream `await` range - `"SELECT ..." + " WHERE ..." + req.query.message`. -- Allows a source contained inside a concat sink to reach that sink when the - sink text contains `+`; this is intentionally narrower than globally - allowing contained token sources. -- Adds smoke coverage: - `scan JavaScript taint Express request reaches SQL string concat`. -- Closes upstream JS tainting-rule case `await` and the missing direct concat - finding in `simpl_nodejs_eval`. -- JS tainting-rule sweep became 8 passed / 3 mismatched. - -`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 - backtick. -- Adjusts `bind-metavariable` in `src/semgrep/match/structural.ss` so - unquoted metavariables bound to `template_string` nodes use the same trimmed - end range and binding text. -- Keeps the existing quoted-template content binding behavior intact. -- Adds smoke coverage: - `scan JavaScript template metavariable ranges omit closing backtick`. -- 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: - -- Makes taint-state deduplication distinguish states by metavariable binding - values, so `source` can retain separate `$X = A` and `$X = B` alternatives. -- Renders ordinary taint sink findings once per reaching source state, with the - reaching source bindings taking precedence over same-named sink bindings for - message rendering. -- Preserves the sink finding's existing extra fields while replacing metavars, - so taint sinks do not accidentally gain source-rendered `fix` or `fix-regex` - output. -- Adds smoke coverage: - `scan JavaScript taint preserves conditional source metavariables`. -- Closes upstream JS tainting-rule case `metavar_eq_conditional`. -- JS tainting-rule sweep is now 11 passed / 0 mismatched. - -Latest checkpoint: - -- Adds Dart impossible-catch filtering for taint findings inside `catch` - blocks whose matching `try` body returns a simple non-throwing expression. -- Adds conservative brace-style function scope detection so Dart/C-like - functions do not leak taint across sibling functions or methods. -- Closes upstream Dart tainting-rule case `try_return`. -- Moves the Dart tainting-rule sweep from `0 passed / 2 mismatched / - 0 Jerboa errors` to `1 passed / 1 mismatched / 0 Jerboa errors`. -- Adds smoke coverage: - `scan Dart taint ignores catch after nonthrowing return`. - -Previous checkpoint: - -- Adds Ruby `case`/`when`/`else` sibling-branch detection for taint - reachability. -- Prevents a sanitizer or clean assignment in one Ruby `when` branch from - killing a tainted value that reaches a sink in a sibling `when` or `else` - branch. -- Extends sanitizer blocking, assignment kills, and sanitizer-blocked - propagators to check the sanitizer/assignment against the sink or propagator - site, not only source against assignment. -- Closes upstream Ruby tainting-rule case `switch`. -- Moves the Ruby tainting-rule sweep from `0 passed / 1 mismatched / - 0 Jerboa errors` to `1 passed / 0 mismatched / 0 Jerboa errors`. -- Adds smoke coverage: - `scan Ruby taint keeps sibling case branches separate`. - -Previous checkpoint: - -- Adds a Python/Django taint source filter for rules whose source formula - contains `is_safe_url` and whose sinks are Django redirects. -- Removes source matches inside functions containing an imported - `is_safe_url(...)` call before implicit assignment propagation can carry the - request value to `HttpResponseRedirect(...)`. -- Closes upstream Python tainting-rule case `simpl_django_redirect`. -- Moves the Python tainting-rule sweep from `11 passed / 1 mismatched / - 0 Jerboa errors` to `12 passed / 0 mismatched / 0 Jerboa errors`. -- Adds smoke coverage: - `scan Python taint Django redirect honors imported safe URL guard`. - -Previous checkpoint: - -- Adds Python taint unreachable-tail filtering after `raise`, `return`, - `break`, and `continue`. -- Closes upstream Python tainting-rule cases `break`, `raise_from`, - `raise_no_args`, `try_raise`, and `try_finally1`. -- Moves the Python tainting-rule sweep from `6 passed / 6 mismatched / - 0 Jerboa errors` to `11 passed / 1 mismatched / 0 Jerboa errors`. -- Added smoke coverage: - `scan Python taint filters unreachable suite tails`. - -- Adds a parser normalization pass for YAML lines where an inline comment is - the only mapping value after a colon, for example `patterns: # comment`. -- Keeps that normalization out of block scalar bodies so Semgrep pattern text - is not rewritten. -- Closes upstream Python tainting-rule case `source_param`. -- Moves the Python tainting-rule sweep from `5 passed / 6 mismatched / - 1 Jerboa parse error` to `6 passed / 6 mismatched / 0 Jerboa errors`. -- Adds smoke coverage: - `parse YAML empty mapping value with inline comment`. -- Adds smoke coverage: - `scan Python taint source parameter under inline-comment nested patterns`. - -Latest checkpoint modifies: - -```text -HANDOFF_OPUS_4_8.md -src/semgrep/scan.ss -lib/semgrep/scan.sls -src/.jerbuild-hashes -tests/smoke.ss -``` - -## Latest Code Change Details - -### Dart Try-Return Catch Reachability - -The upstream Dart taint case `try_return` is translated from the Java -`try_return` fixture. The important contrast is: - -```dart -String? test1() { - String str = "safe"; - try { - return couldThrow(); - } catch (e) { - str = source(); - } - sink(str); // report: couldThrow() may throw -} - -int? test2() { - int cannotThrow = 42; - String str = "safe"; - try { - return cannotThrow; - } - catch (e) { - str = source(); - } - sink(str); // no report: catch is dead -} -``` - -Jerboa previously reported both sinks. Two issues were involved: - -- The `catch` body after `return cannotThrow;` was treated as reachable, so the - `source()` assignment could taint `str`. -- Dart methods were not recognized by `same-simple-function-scope?`, so the - reachable source in `test1` could also flow into the sink in `test2`. - -The patch adds these helpers in -[src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): - -- `dart-significant-trimmed-line` -- `dart-control-line-normalized` -- `dart-try-line?` -- `dart-catch-line?` -- `dart-parent-catch-clause` -- `dart-matching-try-line` -- `dart-return-expression` -- `dart-simple-nonthrowing-expression?` -- `dart-try-body-outcome` -- `dart-finding-in-impossible-catch-branch?` -- `filter-dart-taint-reachable-findings` - -`filter-taint-reachable-findings` now composes the Dart filter with the -existing Python and JavaScript reachability filters. The Dart filter is -conservative: it only treats returns of identifiers, `null`, booleans, quoted -strings, and numeric literals as non-throwing. It intentionally leaves -`return couldThrow();` as unknown/reachable. - -The patch also adds: - -- `simple-c-like-function-line?` - -and includes it in `simple-function-scope-line?`. This recognizes -brace-style function declarations such as: - -```dart -String? test1() { -void F(Object? x) { -``` - -and also helps other C-like languages avoid cross-function taint leakage. It -explicitly excludes common control-flow headers such as `if`, `for`, `while`, -`switch`, and `catch`. - -Smoke coverage added: - -```scheme -(test-case "scan Dart taint ignores catch after nonthrowing return" - ...) -``` - -Focused upstream verification: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -Full Dart taint verification: - -```text -upstream-sweep: 1 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 2 compared -``` - -Remaining Dart mismatch: - -```text -arrays_if -``` - -### Ruby Case Branch Taint Reachability - -The upstream Ruby taint case `switch` is: - -```ruby -def f() - i = source() - sink(i) # report - case x - when 0 - i = sanitize(i) - sink(i) # no report - when 1 - sink(i) # report - else - sink(i) # report - end -end -``` - -The rule is the normal taint shape: - -```yaml -pattern-sources: - - pattern: source(...) -pattern-sanitizers: - - pattern: sanitize(...) -pattern-sinks: - - pattern: sink(...) -``` - -Jerboa previously reported only the first sink. The `i = sanitize(i)` finding -inside `when 0` was path-insensitively treated as both: - -- a sanitizer between the original propagated `i` source and later sinks, and -- a clean assignment kill for the implicit assignment source state carried by - `i`. - -That was correct for the sink in the same branch, but wrong for sibling -branches. Semgrep treats Ruby `when`/`else` clauses in the same `case` as -mutually exclusive, so the sanitizer in `when 0` cannot clean the paths through -`when 1` or `else`. - -The patch adds these line-oriented helpers in -[src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): - -- `ruby-significant-trimmed-line` -- `ruby-keyword-line?` -- `ruby-case-line?` -- `ruby-case-clause-kind` -- `ruby-parent-case-clause` -- `ruby-case-chain-start` -- `ruby-case-branch-info` -- `ruby-findings-in-mutually-exclusive-case-branches?` - -`findings-in-mutually-exclusive-if-branches?` now also delegates to the Ruby -case helper. The name is now a little narrow, but it preserves the existing call -sites and keeps this checkpoint small. - -Three taint reachability sites now avoid cross-branch suppression: - -- `sanitizer-blocks?` checks whether the sanitizer and sink are in sibling - mutually exclusive branches. -- `taint-assignment-kills-source?` checks whether the assignment kill and sink - are in sibling mutually exclusive branches. This is necessary when the source - is outside the `case` but the clean assignment is inside one branch. -- `propagator-blocked-by-sanitizer?` checks whether the sanitizer and - propagator source site are in sibling mutually exclusive branches. - -The Ruby helper is intentionally scoped and text based. It walks upward from a -finding to the nearest less-indented `when` or `else` clause, then walks further -up to the matching same-indentation `case`. Two findings are mutually exclusive -only when they have the same case start, the same clause indentation, and -different clause starts. This handles the upstream fixture and avoids changing -the broader taint model into a full control-flow graph. - -Smoke coverage added: - -```scheme -(test-case "scan Ruby taint keeps sibling case branches separate" - ...) -``` - -Focused upstream verification: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -Full Ruby taint verification: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -Shared taint regressions checked: - -```text -tests/tainting_rules/python: 12 passed, 0 mismatched, 0 Jerboa errors -tests/tainting_rules/js: 11 passed, 0 mismatched, 0 Jerboa errors -``` - -### Django Safe URL Taint Source Guard - -The upstream Python taint case `simpl_django_redirect` has sources shaped like: - -```yaml -pattern-sources: - - patterns: - - pattern-inside: | - def $FUNC(...): - ... - - pattern-not-inside: | - def $FUNC(...): - ... - <... django.utils.http.is_safe_url(...) ...> - ... - - pattern-either: - - pattern: request.$W.get(...) - - pattern: request.$W(...) - - pattern: request.$W[...] - - metavariable-regex: - metavariable: $W - regex: (?!get_full_path) -pattern-sinks: - - pattern-either: - - pattern: django.shortcuts.redirect(...) - - pattern: django.http.HttpResponseRedirect(...) -``` - -The target imports the guard: - -```python -from django.utils.http import is_safe_url -``` - -and then uses it before the redirect: - -```python -def url_validation2(request): - next = request.POST.get('next', request.GET.get('next')) - ok = is_safe_url(url=next, allowed_hosts=request.get_host()) - if ok: - response = HttpResponseRedirect(next) if next else HttpResponse(status=204) -``` - -Semgrep suppresses that function because the source is inside a function with -an `is_safe_url(...)` guard. Jerboa previously let the source at line 39 -propagate through the implicit assignment to `next`, then reported the sink at -line 43. - -The patch adds: - -- `taint-value-contains-string?` -- `taint-specs-contain-string?` -- `taint-django-safe-url-source-filter?` -- `python-def-line?` -- `python-previous-def-first` -- `python-finding-in-function-containing?` -- `source-match-in-python-safe-url-function?` -- `filter-python-django-safe-url-source-matches` - -`scan-taint-rule*` now computes `raw-source-matches` first and then applies the -new filter for Python taint rules whose source specs mention `is_safe_url` and -whose sink specs mention Django redirect APIs. The filter removes source -matches whose enclosing function body contains `is_safe_url`, which prevents -later implicit assignment propagation from producing the overreported sink. - -This deliberately mirrors the existing search-mode -`python-open-redirect-function-safe?` fallback in the taint pipeline. A more -general future improvement would make `pattern-not-inside` itself -FQN/import-aware for `django.utils.http.is_safe_url(...)`, but this checkpoint -keeps the change scoped to the Django redirect taint rule shape that exposed -the gap. - -Smoke coverage added: - -```scheme -(test-case "scan Python taint Django redirect honors imported safe URL guard" - ...) -``` - -Focused upstream verification: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -### Python Taint Unreachable Suite Tails - -Semgrep does not report taint findings in Python statements that are -syntactically after an abrupt exit in the same suite. The upstream fixtures -exposed this in several small cases: - -```python -def f(): - while (i < 0): - if i == 4: - sink(source) # report - break - sink(source) # do not report -``` - -and: - -```python -def foo(): - try: - raise Foo() - sink(source) # do not report - except: - sink(source) # report - sink(source) # report -``` - -Jerboa already had a Python taint filter for impossible `try`/`except`/`else` -branches, but it did not filter ordinary unreachable tails within the current -suite. The patch adds: - -- `python-line-starts-keyword?` -- `python-taint-abrupt-exit-line?` -- `python-taint-unreachable-at-indent?` -- `python-taint-unreachable-finding?` - -`filter-python-taint-reachable-findings` now removes findings when either the -new unreachable-tail predicate or the existing impossible-exception-branch -predicate is true. - -The unreachable-tail scan is indentation scoped. Once it sees `raise`, -`return`, `break`, or `continue`, later findings at the same or deeper -indentation are filtered. A later significant line with a lower indentation -clears the unreachable state, so sinks after a loop or after an `except` block -remain reachable. - -Smoke coverage added: - -```scheme -(test-case "scan Python taint filters unreachable suite tails" - ...) -``` - -Focused upstream verification: - -```text -upstream-sweep: 6 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 6 compared -``` - -### YAML Empty Mapping Inline Comments - -The upstream Python taint case `source_param` uses this source formula: - -```yaml -pattern-sources: - - patterns: - - pattern-inside: | - source1($SRC, ...) - ... - - patterns: # HACK to disable `pattern: $X` optim - - pattern: $SRC -``` - -The YAML is valid: the second `patterns:` key has no scalar value, and the -text after `#` is a comment. Jerboa's bundled `(std text yaml)` reader does -not implement that inline-comment behavior. A direct probe showed: - -```scheme -(yaml-load-string "patterns: # HACK\n - pattern: $SRC\n") -``` - -returning this shape: - -```scheme -((("patterns" . "# HACK")) ((("pattern" . "$SRC")))) -``` - -`parse-positive-pattern` then saw the `patterns` value as the scalar -`"# HACK"` instead of the nested list, and failed with: - -```text -Exception in parse-config-string: patterns must be a nonempty list -``` - -The patch adds `normalize-yaml-empty-value-comments` before the existing plain -scalar and sequence-indentation normalizers. It rewrites only lines where the -first non-space character after a mapping colon is `#`; for example: - -```yaml -patterns: # comment -``` - -becomes: - -```yaml -patterns: -``` - -The helper tracks simple block-scalar contexts so pattern bodies under `|` or -`>` are not altered if they contain text shaped like `foo: # bar`. This is -important because Semgrep configs frequently embed target-language code in -block scalars, and that code can contain comments or YAML-looking text that is -not YAML syntax. - -Relevant code locations in -[src/semgrep/rule/parse-rule.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/rule/parse-rule.ss): - -- `block-scalar-indicator-tail?` -- `line-starts-block-scalar?` -- `empty-mapping-value-comment-index` -- `strip-empty-mapping-value-comment` -- `normalize-yaml-empty-value-comments` -- `parse-config-string`, where the new normalizer is composed before - `normalize-yaml-plain-scalar-continuations` and - `normalize-yaml-sequence-indentation`. - -Smoke coverage added: - -```scheme -(test-case "parse YAML empty mapping value with inline comment" - ...) - -(test-case "scan Python taint source parameter under inline-comment nested patterns" - ...) -``` - -Focused upstream verification: - -```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared -``` - -### Conditional Source Metavariable Environments - -The upstream `metavar_eq_conditional` taint case is: - -```javascript -var source = cond ? get(A) : get(B) - -sink(A, source) -sink(B, source) -``` - -with: - -```yaml -pattern-sources: - - pattern: get($X) -pattern-sinks: - - pattern: sink($X,...) -message: Matched on $X! -``` - -Semgrep reports both `Matched on A!` and `Matched on B!` at both sink calls, -because `source` may carry either source metavariable environment. Jerboa -previously collapsed the two propagated states for token `source` as duplicate -taint states, and ordinary taint sink output always returned the sink finding's -own metavariables. - -This checkpoint changes two pieces: - -- `taint-state-already-present?` now compares metavariable binding values on - the taint state's finding. Same carrier range plus different bindings is no - longer treated as a duplicate state. -- `sink-output-findings` now renders non-`taint_focus_on: source` sink output - per reaching source state, with source bindings taking precedence over - same-named sink bindings. This lets line 9 render both `A` and `B` messages. - -Important implementation details: - -- `finding-with-message-and-extra` creates a finding with a re-rendered message - while preserving the existing sink extra fields. -- `extra-with-metavars` replaces only the displayed metavars. It intentionally - does not call `finding-extra-for-match`, because that would generate - source-rendered `fix`/`fix-regex` output on taint sinks. The broad - `taint_param_default` case caught this during validation. -- `binding-list-with-source-precedence` keeps sink-only bindings available but - lets source bindings win when a name appears in both places. - -Relevant code locations in [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): - -- `finding-with-message-and-extra`: near the finding helpers around line 530. -- `binding-list-same-values?` and `finding-bindings-same-values?`: near - `taint-state-already-present?`. -- `taint-output-finding-for-source`: near `sink-output-findings`. - -Smoke coverage added: - -```scheme -(test-case "scan JavaScript taint preserves conditional source metavariables" - ...) -``` - -The smoke asserts four findings: `matched A` and `matched B` at both -`sink(A, source)` and `sink(B, source)`. - -### 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 -opening backtick through the template content, excluding the closing backtick. -For example, with: - -```yaml -patterns: - - pattern-inside: foo($X) - - pattern: $X -``` - -and: - -```javascript -foo(`lol(${req.query.userInput})`); -``` - -Semgrep reports columns 5 to 33, not 5 to 34. Jerboa previously let structural -`$X` bind to the entire `template_string` node, so both `patterns` intersections -and `focus-metavariable` included the closing backtick. - -The scanner now trims JavaScript `template_string` structural findings to -`node-end-byte - 1`, while the structural matcher trims unquoted template-string -metavariable bindings the same way. This fixed `simpl_nodejs_eval`, whose final -mismatch was the sink on: - -```javascript -s.run(`lol(${req.query.userInput})`, cb); -``` - -Semgrep expects the focused sink to stop before the closing backtick. - -### JavaScript SQL String Concat Taint - -The upstream `await` taint rule uses this sink: - -```yaml -pattern-sinks: - - pattern: | - "$SQLSTR" + $EXPR -``` - -Semgrep matches the associative concat range through the first non-literal -operand: - -```javascript -"SELECT * FROM users" + " WHERE id = '" + req.query.message -``` - -Jerboa previously matched only the first binary fragment: - -```javascript -"SELECT * FROM users" + " WHERE id = '" -``` - -That excluded `req.query.message`, so taint did not reach the sink. The scanner -now has a JavaScript concat fallback for quoted-string metavariable patterns -that: - -- recognizes `"$NAME" + $EXPR` / `'$NAME' + $EXPR` / `` `$NAME` + $EXPR ``; -- keeps SQLSTR starts limited to strings containing common SQL verbs, avoiding - later literal fragments like `" WHERE ..."`; -- binds `$EXPR` from after the first `+` through the first non-literal operand; -- works in the binding-preserving formula path used by taint specs. - -Taint reachability also has a narrow contained-source branch for concat sinks: -if the source occurrence is contained inside a sink whose text includes `+`, -that source can reach the sink. This avoids the broader token-source change -that overreported typestate cases. - -This closed `await` and also added the missing direct concat report for -`simpl_nodejs_eval`; the remaining template-literal range mismatch was closed -by the later template-string range checkpoint. - -### Scoped Trailing-Ellipsis `pattern-inside` - -Semgrep treats a JavaScript `pattern-inside` such as: - -```yaml -pattern-inside: | - $JWT.verify($TOKEN, ...) - ... -``` - -as a region after the verifier inside the relevant lexical block. Jerboa's -fallback for this shape previously checked only that the base match appeared -before the candidate. That let a verifier in one function act as context for a -candidate in a later function. - -The upstream `sanitized_by_side_effect` rule exposes this: - -```javascript -const jwt = require('jsonwebtoken'); - -function ok(token, key) { - jwt.verify(token, key); - jwt.decode(token, true); // sanitized -} - -const ok2 = (token, key) => { - jwt.verify(token, key); - jwt.decode(token, true); // sanitized -}; - -function bad_different_token(token, key) { - token2 = getToken(); - jwt.verify(token2, key); - jwt.decode(token, true); // must report -} -``` - -Jerboa was incorrectly allowing the verifier from `ok2` to satisfy the -sanitizer `pattern-inside` for the later `bad_different_token` sink. - -The scanner now: - -- Tracks the nearest unmatched `{` before the base finding, ignoring braces in - JavaScript strings and line/block comments. -- Finds the corresponding close brace with `javascript-matching-close-brace`. -- Falls back to EOF when the base finding is top-level. -- Requires the candidate's end offset to be within that scope before merging - metavariable bindings in `inside-after-clause-apply`. - -This keeps top-level trailing-ellipsis context such as -`$JWT = require('jsonwebtoken'); ...` file-wide, while preventing a verifier in -one function body from sanitizing a token in a later function. - -### Previous Checkpoint: Propagated Taint Unification - -The previous code checkpoint fixed `metavar_eq_simple`. The upstream shape was: - -```javascript -var source1 = get(A) -var source2 = get(B)