Clear destructuring taint frontier
ober
38bfcf1bece11c51526a8290516c8e2a221703c6
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,6 +1,6 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-28 23:57 MDT local workspace time +Date: 2026-05-29 00:15 MDT local workspace time Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` @@ -31,6 +31,7 @@ taint/dataflow, path and target semantics, autofix, and output schemas. Recent checkpoints before this handoff commit: ```text +19f45f7 Advance lambda and source taint parity 7cfb751 Clear lambda taint label reach 31db293 Clear taint label branch frontier 34227c0 Clear final global taint frontier @@ -73,7 +74,7 @@ make test Result: ```text -191 tests, 191 passed, 0 failed +194 tests, 194 passed, 0 failed ``` Local oracle: @@ -88,10 +89,10 @@ Result: oracle: 42 passed, 0 failed ``` -Focused lambda, source-focus, and nearby taint guardrail for this checkpoint: +Focused destructuring and assignment-only taint guardrail for this checkpoint: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_lambda2|taint_lambda4|taint_match_on_source|taint_typestate|taint_side_effect|taint_source)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_nested_record_pattern|taint_no_builtin_props|taint_no_builtin_props1|taint_object_destructure)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh ``` Result: @@ -100,8 +101,8 @@ Result: upstream-sweep: 4 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 4 compared ``` -This confirms `taint_lambda2`, `taint_lambda4`, `taint_match_on_source`, -`taint_typestate`, `taint_side_effect`, and `taint_source` are clean in the +This confirms `taint_nested_record_pattern`, `taint_no_builtin_props`, +`taint_no_builtin_props1`, and `taint_object_destructure` are clean in the current tree. Focused upstream guardrail containing the field-sensitive cases fixed so far: @@ -149,7 +150,7 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep MAX_CASES=220 LIST_MISMATCHES=1 M Result: ```text -upstream-sweep: 201 passed, 17 mismatched, 0 jerboa errors, 2 current errors, 220 compared +upstream-sweep: 205 passed, 13 mismatched, 0 jerboa errors, 2 current errors, 220 compared ``` The full 241-case sweep was not rerun after this checkpoint. Do not treat the @@ -172,27 +173,30 @@ errors. ## What Changed In This Checkpoint -This checkpoint clears `taint_lambda4` and `taint_match_on_source`, building -on `7cfb751` which cleared `taint_lambda2`. +This checkpoint clears `taint_nested_record_pattern`, `taint_no_builtin_props`, +`taint_no_builtin_props1`, and `taint_object_destructure`, building on +`19f45f7` which cleared `taint_lambda4` and `taint_match_on_source`. Implementation changes in `src/semgrep/scan.ss`: -- JS/TS `pattern-inside: function ... ($REQ) {...}` now has a targeted - fallback that recognizes ordinary function declarations and arrow functions - with typed parameters such as `req: Request`. -- JS/TS positive pattern matching now handles the expression source shape - `$X.$Y. ... + foobar` against member-chain additions such as - `req.query + foobar`, preserving `$X` and `$Y` bindings for later focus and - source `requires`. -- Taint states now preserve their original source finding through propagation. - This lets `options: taint_focus_on: source` report the source match even when - the sink is reached through assignments. -- Taint output selection now uses that source origin only when the rule option - explicitly requests source focus; ordinary taint rules still report sinks. +- JS/TS implicit assignment propagation now includes one-level nested object + destructuring declarations such as `const { body: { param } } = tainted`. +- Built-in implicit assignment propagators now honor + `taint_only_propagate_through_assignments`: when the option is set, a + built-in assignment propagates only if the RHS binding text is exactly the + current source text. This preserves `x = source(); y = x` while suppressing + propagation through `x.a`, `x()`, `y + "ok"`, `f(y)`, and `a[y]`. +- JS arrow function source matching now handles `({ $X }, $A) => $Z` and + emits one focused source for each destructured object parameter property. +- The arrow destructuring fallback exports only the focused `$X` binding so the + sibling parameter `$A` does not become tainted. New smoke coverage in `tests/smoke.ss`: ```text +scan JavaScript taint nested object destructuring assignment +scan taint only propagates through plain assignments +scan JavaScript taint destructured arrow object parameters scan TypeScript taint combines typed lambda source labels scan taint focus on source reports original source scan JavaScript taint combines lambda db and request labels @@ -214,6 +218,80 @@ subtrees. The ancestor-clean test mirrors `taint_field_sensitive4`: ## Resolved Recent Cases +`taint_nested_record_pattern` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_nested_record_pattern$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +``` + +Current result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +What changed: + +- Target file: + `/Users/user/mine/semgrep/tests/rules/taint_nested_record_pattern.js`. +- Shape: + `const { body: { param } } = tainted; sink(param)`. +- Jerboa already supported flat JS object destructuring. The implicit + assignment pattern list now also includes nested shorthand object patterns + for `const`, `let`, and `var`. +- The propagated binding is the nested leaf identifier, so `param` receives + taint from the RHS object while unrelated names stay clean. + +`taint_no_builtin_props` and `taint_no_builtin_props1` are now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_no_builtin_props|taint_no_builtin_props1)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +``` + +Current result: + +```text +upstream-sweep: 2 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 2 compared +``` + +What changed: + +- Both rules use `options: taint_only_propagate_through_assignments: true`. +- Jerboa previously reported expected assignment-only flows plus false + positives through expression RHS values such as `y + "ok"`, `f(y)`, + `a[y]`, `x.a`, and `x()`. +- Built-in implicit assignment propagators are now tagged and blocked under + this option unless the RHS binding text exactly matches the current source + finding text. +- Explicit user `pattern-propagators` are not tagged as built-in implicit + assignments and are left to their own rule semantics. + +`taint_object_destructure` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_object_destructure$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +``` + +Current result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +What changed: + +- Target file: + `/Users/user/mine/semgrep/tests/rules/taint_object_destructure.js`. +- Rule source shape: + `({ $X }, $A) => $Z` focused to `$X`. +- Jerboa previously returned no source matches for destructured object + parameters in arrow functions. +- The new JS arrow object-parameter fallback finds object properties in the + first destructured parameter and emits one match per property. Focusing `$X` + produces sources for `x` and `y`, which then reach `sink(x)` and `sink(y)`. +- The fallback intentionally does not export the sibling `$A` binding in the + resulting source finding; exporting `$A=z` caused `sink(z)` to become tainted. + `taint_lambda4` is now cleared: ```sh @@ -627,13 +705,9 @@ What changed: ## Current First-220 Frontier -The current first-220 sweep has these 17 mismatches: +The current first-220 sweep has these 13 mismatches: ```text -taint_nested_record_pattern -taint_no_builtin_props -taint_no_builtin_props1 -taint_object_destructure taint_object_funcdef taint_object_funcdef_1 taint_param_default @@ -666,20 +740,21 @@ vardef_assign_true1 vardef_assign_true2 ``` -## Immediate Next Case: taint_nested_record_pattern +## Immediate Next Case: taint_object_funcdef Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_nested_record_pattern$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_object_funcdef$' LIST_MISMATCHES=1 MAX_DIFFS=320 tests/oracle/upstream-sweep.sh ``` Current result: ```text -MISMATCH taint_nested_record_pattern -@@ -1 +0,0 @@ --(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_nested_record_pattern.js" 4 3 76 4 14 87 "ERROR" "Test" "") +MISMATCH taint_object_funcdef +@@ -1,2 +0,0 @@ +-(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_object_funcdef.js" 17 31 358 17 38 365 "ERROR" "Matched SQL injection." "") +-(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_object_funcdef.js" 6 31 157 6 38 164 "ERROR" "Matched SQL injection." "") upstream-sweep: 0 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` @@ -688,58 +763,84 @@ Rule: ```yaml rules: - id: test - message: Test + message: Matched SQL injection. + languages: + - javascript + severity: ERROR mode: taint pattern-sources: - - pattern: tainted + - patterns: + - pattern-either: + - pattern-inside: $APP.$METHOD(..., function $FUNC($REQ, $RES) {...}) + - metavariable-regex: + metavariable: $METHOD + regex: ^(get|post|put|head|delete|options)$ + - pattern: $REQ.params pattern-sinks: - - pattern: sink(...) - severity: ERROR - languages: - - javascript - - typescript + - patterns: + - pattern-either: + - patterns: + - pattern-either: + - pattern-inside: | + "$SQLSTR" + $EXPR + - pattern-inside: | + "$SQLSTR".concat($EXPR) + - pattern: util.format($SQLSTR, $EXPR) + - pattern: | + `$SQLSTR${$EXPR}...` + - metavariable-regex: + metavariable: $SQLSTR + regex: .*\b(?i)(select|delete|insert|create|update|alter|drop)\b.* + - focus-metavariable: $EXPR ``` Target: ```text -/Users/user/mine/semgrep/tests/rules/taint_nested_record_pattern.js +/Users/user/mine/semgrep/tests/rules/taint_object_funcdef.js ``` Relevant target: ```javascript -function test() { - const { body: { param } } = tainted - sink(param) -} +app.get('/ok', async (req, res) => { + const tainted = req.params; + async.parallel({ + somefunc(cb) { + return `SELECT * FROM ${tainted}` + } + }) +}) ``` Packaged Semgrep expects: ```text -4 +6, 17 ``` Jerboa currently reports no findings: ```diff -@@ -1 +0,0 @@ --(finding "test" ".../taint_nested_record_pattern.js" 4 3 ... "ERROR" "Test" "") -``` - -Interpretation for the next fix: - -- This is likely missing JavaScript nested object destructuring propagation: - `const { body: { param } } = tainted` should taint `param`. -- Existing JavaScript destructuring support already handles simpler object - destructuring shapes; inspect `scan-javascript-destructuring` and implicit - assignment propagator construction around `src/semgrep/scan.ss`. -- Add a smoke test that uses `const { body: { param } } = tainted` with source - `tainted` and sink `sink(...)`, then make the destructuring alias collector - descend through nested object patterns. -- Guard against over-tainting sibling destructured names; the source is the RHS - object, and the propagated binding should be the nested leaf identifier. +@@ -1,2 +0,0 @@ +-(finding "test" ".../taint_object_funcdef.js" 17 31 ... "ERROR" "Matched SQL injection." "") +-(finding "test" ".../taint_object_funcdef.js" 6 31 ... "ERROR" "Matched SQL injection." "") +``` + +Isolation already done: + +- Simple `pattern: $REQ.params` finds `req.params` and binds `$REQ=req`. +- The full source formula returns no findings. The likely missing source side + is `pattern-inside: $APP.$METHOD(..., function $FUNC($REQ, $RES) {...})` + matching Express arrow callbacks like `app.get('/ok', async (req, res) => { ... })`. +- The sink formula also returns no findings for + ``return `SELECT * FROM ${tainted}` ``. The likely missing sink side is + template-string support for the pattern `` `$SQLSTR${$EXPR}...` `` focused + to `$EXPR`, with `$SQLSTR` satisfying the SQL keyword regex. +- Fix this in two small pieces if possible: first make the route callback + context produce the focused source, then make the SQL template sink produce + a focused `$EXPR` finding. Add smoke coverage that expects `tainted` inside + the template string to be the reported sink range. ## Resolved Earlier Frontier --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -4227,6 +4227,12 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-arrow-object-param-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) (scan-javascript-member-ellipsis-plus-pattern rule path @@ -4417,6 +4423,8 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-arrow-object-param-pattern-with-bindings rule path source (cdr entry) initial-bindings)) + (and (javascript-like-language? language) (scan-javascript-member-ellipsis-plus-pattern-with-bindings rule path source (cdr entry) initial-bindings)) (and (javascript-like-language? language) (scan-javascript-new-function-sequence-pattern @@ -5432,6 +5440,182 @@ source pattern) (scan-javascript-member-ellipsis-plus-pattern-with-bindings rule path source pattern '())) + (def (js-arrow-object-param-pattern-spec pattern) + (let* ([trimmed (string-trim pattern)] + [match (re-search + (re "^\\([ \t\n]*\\{[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)[ \t\n]*\\}[ \t\n]*,[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)[ \t\n]*\\)[ \t\n]*=>[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)$") + trimmed + 0)]) + (and match + (list + (cons 'object-metavariable (re-match-group match 1)) + (cons 'arg-metavariable (re-match-group match 2)) + (cons 'body-metavariable (re-match-group match 3)))))) + (def (js-skip-horizontal-and-newline source i end) + (let loop ([j i]) + (if (and (< j end) (char-whitespace? (string-ref source j))) + (loop (+ j 1)) + j))) + (def (js-find-next-comma-or-end source i end) + (let loop ([j i]) + (cond + [(>= j end) end] + [(char=? (string-ref source j) #\,) (+ j 1)] + [else (loop (+ j 1))]))) + (def (js-object-param-binding-target-range source start end) + (let* ([key-end (access-path-identifier-end source start)] + [after-key (js-skip-horizontal-and-newline + source + key-end + end)]) + (if (and (< after-key end) + (char=? (string-ref source after-key) #\:)) + (let* ([value-start (js-skip-horizontal-and-newline + source + (+ after-key 1) + end)] + [value-end (and (< value-start end) + (identifier-char? + (string-ref source value-start)) + (access-path-identifier-end + source + value-start))]) + (if value-end + (cons value-start value-end) + (cons start key-end))) + (cons start key-end)))) + (def (js-object-param-bindings + source + start + end + metavariable) + (let loop ([i start] [acc '()]) + (let ([j (js-skip-horizontal-and-newline source i end)]) + (cond + [(>= j end) (reverse acc)] + [(char=? (string-ref source j) #\,) (loop (+ j 1) acc)] + [(identifier-char? (string-ref source j)) + (let* ([range (js-object-param-binding-target-range + source + j + end)] + [binding-start (car range)] + [binding-end (cdr range)] + [binding (make-regex-capture-binding metavariable + (substring + source + binding-start + binding-end) + source binding-start binding-end)] + [next (js-find-next-comma-or-end + source + binding-end + end)]) + (loop next (cons binding acc)))] + [else + (loop (js-find-next-comma-or-end source j end) acc)])))) + (def (js-arrow-object-param-targets source match) + (let* ([start (re-match-start match)] + [full (re-match-full match)] + [object-content (re-match-group match 1)] + [arg (re-match-group match 2)] + [object-open-rel (or (string-find-substring-from full "{" 0) + 0)] + [object-start (+ start object-open-rel 1)] + [object-end (+ object-start (string-length object-content))] + [arg-rel (or (string-find-substring-from + full + arg + (+ object-open-rel + (string-length object-content))) + 0)] + [arg-start (+ start arg-rel)] + [arg-end (+ arg-start (string-length arg))] + [arrow-rel (string-find-substring-from full "=>" 0)] + [arrow-start (and arrow-rel (+ start arrow-rel))] + [body-start (and arrow-start + (skip-whitespace source (+ arrow-start 2)))] + [body-end (and body-start + (if (and (< body-start (string-length source)) + (char=? + (string-ref source body-start) + #\{)) + (find-matching-close-brace + source + body-start) + (line-end-after source body-start)))]) + (and body-start + body-end + (list (cons 'match-start start) (cons 'match-end body-end) + (cons 'object-start object-start) + (cons 'object-end object-end) (cons 'arg-start arg-start) + (cons 'arg-end arg-end) (cons 'body-start body-start) + (cons 'body-end body-end))))) + (def (js-arrow-object-param-finding-for-binding rule path source targets spec object-binding + initial-bindings) + (let* ([object-metavariable (alist-ref/default + spec + 'object-metavariable + #f)] + [match-start (alist-ref/default targets 'match-start 0)] + [match-end (alist-ref/default targets 'match-end 0)] + [merged (merge-binding-list + initial-bindings + (list (cons object-metavariable object-binding)))]) + (and merged + (finding-for-range-with-bindings rule path source + match-start match-end merged)))) + (def (scan-javascript-arrow-object-param-pattern-with-bindings rule path source pattern initial-bindings) + (let ([spec (js-arrow-object-param-pattern-spec pattern)]) + (and spec + (let ([rx (re "\\(\\s*\\{([^}]*)\\}\\s*,\\s*([A-Za-z_$][A-Za-z0-9_$]*)\\s*\\)\\s*=>")] + [len (string-length source)] + [object-metavariable (alist-ref/default + spec + 'object-metavariable + #f)]) + (let loop ([offset 0] [acc '()]) + (if (> offset len) + (nonempty-findings (reverse acc)) + (let ([match (re-search rx source offset)]) + (if match + (let* ([targets (js-arrow-object-param-targets + source + match)] + [object-bindings (if targets + (js-object-param-bindings + source + (alist-ref/default + targets + 'object-start + 0) + (alist-ref/default + targets + 'object-end + 0) + object-metavariable) + '())] + [findings (if targets + (sg-filter + (lambda (finding) + finding) + (map (lambda (binding) + (js-arrow-object-param-finding-for-binding rule path + source targets + spec binding + initial-bindings)) + object-bindings)) + '())] + [next (max (+ (re-match-start match) 1) + (re-match-end match))]) + (loop next (append (reverse findings) acc))) + (nonempty-findings (reverse acc)))))))))) + (def (scan-javascript-arrow-object-param-pattern + rule + path + source + pattern) + (scan-javascript-arrow-object-param-pattern-with-bindings rule path source pattern '())) (def (js-nearest-recursive-context-start source offset) (let loop ([xs (js-recursive-contexts-before source offset)] [best #f]) @@ -8471,7 +8655,8 @@ (alist-ref/default propagator 'replace-labels - #f)))) + #f)) + (cons 'implicit-assignment #f))) (filter-python-taint-reachable-findings language source @@ -8489,7 +8674,9 @@ '("$L = $R" "$L[$I] = $R" "const $L = $R" "let $L = $R" "var $L = $R" "const { $L } = $R" "let { $L } = $R" "var { $L } = $R" "const { $FIELD: $L } = $R" "let { $FIELD: $L } = $R" - "var { $FIELD: $L } = $R")] + "var { $FIELD: $L } = $R" "const { $FIELD: { $L } } = $R" + "let { $FIELD: { $L } } = $R" + "var { $FIELD: { $L } } = $R")] [(or (string=? language "python") (string=? language "python2") (string=? language "python3") @@ -8505,7 +8692,8 @@ (list (cons 'finding finding) (cons 'from "$R") (cons 'to "$L") (cons 'by-side-effect #t) (cons 'label #f) (cons 'requires #f) - (cons 'replace-labels #f))) + (cons 'replace-labels #f) + (cons 'implicit-assignment #t))) (filter-python-taint-reachable-findings language source @@ -8755,6 +8943,10 @@ (rule-option-enabled? rule "taint_assume_safe_indexes")) (def (taint-assume-safe-functions? rule) (rule-option-enabled? rule "taint_assume_safe_functions")) + (def (taint-only-propagate-through-assignments? rule) + (rule-option-enabled? + rule + "taint_only_propagate_through_assignments")) (def (taint-focus-on-source? rule) (let ([value (rule-option-enabled? rule "taint_focus_on")]) (or (eq? value 'source) @@ -9693,6 +9885,19 @@ sanitizer source))))))) sanitizers)) + (def (same-trimmed-finding-text? a b source) + (and a + b + (string=? + (string-trim (finding-text a source)) + (string-trim (finding-text b source))))) + (def (implicit-assignment-propagation-blocked-by-option? rule source-state propagator from-finding source) + (and (taint-only-propagate-through-assignments? rule) + (alist-ref/default propagator 'implicit-assignment #f) + (not (same-trimmed-finding-text? + (taint-state-finding source-state) + from-finding + source)))) (def (taint-propagator-blocked-by-safe-option? rule source-state @@ -9751,6 +9956,7 @@ from source) (not (taint-source-killed-before-finding? source-state from-finding assignment-kills propagator source)) + (not (implicit-assignment-propagation-blocked-by-option? rule source-state propagator from-finding source)) (not (propagator-blocked-by-sanitizer? source-state from-finding --- 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" . "998D1E472CAB5367") + ("src/semgrep/scan.ss" . "B020B5A7F3281D03") + ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") - ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") - ("src/semgrep/rule.ss" . "E12C108153C181FA") ("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 @@ -4398,6 +4398,12 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-arrow-object-param-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) (scan-javascript-member-ellipsis-plus-pattern rule path @@ -4639,6 +4645,13 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-arrow-object-param-pattern-with-bindings + rule + path + source + (cdr entry) + initial-bindings)) + (and (javascript-like-language? language) (scan-javascript-member-ellipsis-plus-pattern-with-bindings rule path @@ -5760,6 +5773,200 @@ pattern '())) +(def (js-arrow-object-param-pattern-spec pattern) + (let* ([trimmed (string-trim pattern)] + [match + (re-search + (re "^\\([ \t\n]*\\{[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)[ \t\n]*\\}[ \t\n]*,[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)[ \t\n]*\\)[ \t\n]*=>[ \t\n]*\\$([A-Za-z_][A-Za-z0-9_]*)$") + trimmed + 0)]) + (and match + (list (cons 'object-metavariable (re-match-group match 1)) + (cons 'arg-metavariable (re-match-group match 2)) + (cons 'body-metavariable (re-match-group match 3)))))) + +(def (js-skip-horizontal-and-newline source i end) + (let loop ([j i]) + (if (and (< j end) + (char-whitespace? (string-ref source j))) + (loop (+ j 1)) + j))) + +(def (js-find-next-comma-or-end source i end) + (let loop ([j i]) + (cond + [(>= j end) end] + [(char=? (string-ref source j) #\,) (+ j 1)] + [else (loop (+ j 1))]))) + +(def (js-object-param-binding-target-range source start end) + (let* ([key-end (access-path-identifier-end source start)] + [after-key (js-skip-horizontal-and-newline source key-end end)]) + (if (and (< after-key end) + (char=? (string-ref source after-key) #\:)) + (let* ([value-start + (js-skip-horizontal-and-newline + source + (+ after-key 1) + end)] + [value-end + (and (< value-start end) + (identifier-char? (string-ref source value-start)) + (access-path-identifier-end source value-start))]) + (if value-end + (cons value-start value-end) + (cons start key-end))) + (cons start key-end)))) + +(def (js-object-param-bindings source start end metavariable) + (let loop ([i start] [acc '()]) + (let ([j (js-skip-horizontal-and-newline source i end)]) + (cond + [(>= j end) (reverse acc)] + [(char=? (string-ref source j) #\,) + (loop (+ j 1) acc)] + [(identifier-char? (string-ref source j)) + (let* ([range + (js-object-param-binding-target-range source j end)] + [binding-start (car range)] + [binding-end (cdr range)] + [binding + (make-regex-capture-binding + metavariable + (substring source binding-start binding-end) + source + binding-start + binding-end)] + [next (js-find-next-comma-or-end source binding-end end)]) + (loop next (cons binding acc)))] + [else (loop (js-find-next-comma-or-end source j end) acc)])))) + +(def (js-arrow-object-param-targets source match) + (let* ([start (re-match-start match)] + [full (re-match-full match)] + [object-content (re-match-group match 1)] + [arg (re-match-group match 2)] + [object-open-rel (or (string-find-substring-from full "{" 0) 0)] + [object-start (+ start object-open-rel 1)] + [object-end (+ object-start (string-length object-content))] + [arg-rel + (or (string-find-substring-from + full + arg + (+ object-open-rel (string-length object-content))) + 0)] + [arg-start (+ start arg-rel)] + [arg-end (+ arg-start (string-length arg))] + [arrow-rel (string-find-substring-from full "=>" 0)] + [arrow-start (and arrow-rel (+ start arrow-rel))] + [body-start + (and arrow-start + (skip-whitespace source (+ arrow-start 2)))] + [body-end + (and body-start + (if (and (< body-start (string-length source)) + (char=? (string-ref source body-start) #\{)) + (find-matching-close-brace source body-start) + (line-end-after source body-start)))]) + (and body-start + body-end + (list (cons 'match-start start) + (cons 'match-end body-end) + (cons 'object-start object-start) + (cons 'object-end object-end) + (cons 'arg-start arg-start) + (cons 'arg-end arg-end) + (cons 'body-start body-start) + (cons 'body-end body-end))))) + +(def (js-arrow-object-param-finding-for-binding + rule + path + source + targets + spec + object-binding + initial-bindings) + (let* ([object-metavariable + (alist-ref/default spec 'object-metavariable #f)] + [match-start (alist-ref/default targets 'match-start 0)] + [match-end (alist-ref/default targets 'match-end 0)] + [merged + (merge-binding-list + initial-bindings + (list (cons object-metavariable object-binding)))]) + (and merged + (finding-for-range-with-bindings + rule + path + source + match-start + match-end + merged)))) + +(def (scan-javascript-arrow-object-param-pattern-with-bindings + rule + path + source + pattern + initial-bindings) + (let ([spec (js-arrow-object-param-pattern-spec pattern)]) + (and spec + (let ([rx + (re "\\(\\s*\\{([^}]*)\\}\\s*,\\s*([A-Za-z_$][A-Za-z0-9_$]*)\\s*\\)\\s*=>")] + [len (string-length source)] + [object-metavariable + (alist-ref/default spec 'object-metavariable #f)]) + (let loop ([offset 0] [acc '()]) + (if (> offset len) + (nonempty-findings (reverse acc)) + (let ([match (re-search rx source offset)]) + (if match + (let* ([targets + (js-arrow-object-param-targets source match)] + [object-bindings + (if targets + (js-object-param-bindings + source + (alist-ref/default + targets + 'object-start + 0) + (alist-ref/default + targets + 'object-end + 0) + object-metavariable) + '())] + [findings + (if targets + (sg-filter + (lambda (finding) finding) + (map + (lambda (binding) + (js-arrow-object-param-finding-for-binding + rule + path + source + targets + spec + binding + initial-bindings)) + object-bindings)) + '())] + [next (max (+ (re-match-start match) 1) + (re-match-end match))]) + (loop next (append (reverse findings) acc))) + (nonempty-findings (reverse acc)))))))))) + +(def (scan-javascript-arrow-object-param-pattern rule path source pattern) + (scan-javascript-arrow-object-param-pattern-with-bindings + rule + path + source + pattern + '())) + (def (js-nearest-recursive-context-start source offset) (let loop ([xs (js-recursive-contexts-before source offset)] [best #f]) @@ -8983,7 +9190,8 @@ (cons 'replace-labels (alist-ref/default propagator 'replace-labels - #f)))) + #f)) + (cons 'implicit-assignment #f))) (filter-python-taint-reachable-findings language source @@ -9014,7 +9222,10 @@ "var { $L } = $R" "const { $FIELD: $L } = $R" "let { $FIELD: $L } = $R" - "var { $FIELD: $L } = $R")] + "var { $FIELD: $L } = $R" + "const { $FIELD: { $L } } = $R" + "let { $FIELD: { $L } } = $R" + "var { $FIELD: { $L } } = $R")] [(or (string=? language "python") (string=? language "python2") (string=? language "python3") @@ -9036,7 +9247,8 @@ (cons 'by-side-effect #t) (cons 'label #f) (cons 'requires #f) - (cons 'replace-labels #f))) + (cons 'replace-labels #f) + (cons 'implicit-assignment #t))) (filter-python-taint-reachable-findings language source @@ -9290,6 +9502,9 @@ (def (taint-assume-safe-functions? rule) (rule-option-enabled? rule "taint_assume_safe_functions")) +(def (taint-only-propagate-through-assignments? rule) + (rule-option-enabled? rule "taint_only_propagate_through_assignments")) + (def (taint-focus-on-source? rule) (let ([value (rule-option-enabled? rule "taint_focus_on")]) (or (eq? value 'source) @@ -10196,6 +10411,26 @@ source))))))) sanitizers)) +(def (same-trimmed-finding-text? a b source) + (and a + b + (string=? + (string-trim (finding-text a source)) + (string-trim (finding-text b source))))) + +(def (implicit-assignment-propagation-blocked-by-option? + rule + source-state + propagator + from-finding + source) + (and (taint-only-propagate-through-assignments? rule) + (alist-ref/default propagator 'implicit-assignment #f) + (not (same-trimmed-finding-text? + (taint-state-finding source-state) + from-finding + source)))) + (def (taint-propagator-blocked-by-safe-option? rule source-state @@ -10259,6 +10494,12 @@ assignment-kills propagator source)) + (not (implicit-assignment-propagation-blocked-by-option? + rule + source-state + propagator + from-finding + source)) (not (propagator-blocked-by-sanitizer? source-state from-finding --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1474,6 +1474,18 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 4))) +(test-case "scan taint only propagates through plain assignments" + (let* ([taint-config + "rules:\n - id: demo.taint.assignment.only\n mode: taint\n languages: [python]\n message: assignment-only taint\n severity: WARNING\n options:\n taint_only_propagate_through_assignments: true\n pattern-sources:\n - pattern: source(...)\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "python" + "demo.py" + "def bad():\n x = source()\n y = x\n z = y\n sink(z)\n\ndef ok1():\n x = source()\n y = x.a\n sink(y)\n\ndef ok2():\n x = source()\n y = x()\n sink(y)\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 5))) + (test-case "scan taint sanitizer blocks later assignment propagation" (let* ([taint-config "rules:\n - id: demo.taint.assignment.sanitized-later\n mode: taint\n languages: [python]\n message: sanitized propagation\n severity: WARNING\n pattern-sources:\n - pattern: request\n pattern-sanitizers:\n - pattern: int(...)\n pattern-sinks:\n - pattern: send_file(...)\n"] @@ -1533,6 +1545,30 @@ (check (finding-start-line (car findings)) => 2) (check (finding-start-line (cadr findings)) => 4))) +(test-case "scan JavaScript taint nested object destructuring assignment" + (let* ([taint-config + "rules:\n - id: demo.taint.js.nested.destructure\n mode: taint\n languages: [javascript]\n message: nested destructured taint\n severity: WARNING\n pattern-sources:\n - pattern: tainted\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function test() {\n const { body: { param } } = tainted\n sink(param)\n sink(clean)\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 3))) + +(test-case "scan JavaScript taint destructured arrow object parameters" + (let* ([taint-config + "rules:\n - id: demo.taint.js.arrow.destructure\n mode: taint\n languages: [javascript]\n message: arrow destructured taint\n severity: WARNING\n pattern-sources:\n - patterns:\n - pattern: |\n ({ $X }, $A) => $Z\n - focus-metavariable:\n - $X\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "const foo = ({ x, y }, z) => {\n sink(x);\n sink(y);\n sink(z);\n};\n")]) + (check (length findings) => 2) + (check (map finding-start-line findings) => '(2 3)))) + (test-case "scan TypeScript taint source after trailing pattern-inside" (let* ([taint-config "rules:\n - id: demo.taint.ts.record\n mode: taint\n languages: [typescript]\n message: record taint\n severity: WARNING\n pattern-sources:\n - patterns:\n - pattern-inside: |\n $DBQUERY.findByPk(...)\n ...\n - pattern: $USER.dataValues\n pattern-sinks:\n - patterns:\n - pattern-inside: |\n $PUG = require('pug')\n ...\n - pattern-either:\n - pattern: $PUG.compile(...)\n"]