Advance taint exception and field parity
ober
971b714c7f3e0f7f4d7e97e9c7729319a29b36f8
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -31,6 +31,7 @@ taint/dataflow, path and target semantics, autofix, and output schemas. Recent checkpoints before this handoff: ```text +29dd690 Clear taint exact source frontier 7beb039 Advance taint control parity e429d34 Advance taint safe function parity 70a1319 Advance taint best-fit parity handoff @@ -66,7 +67,7 @@ make test Result: ```text -180 tests, 180 passed, 0 failed +182 tests, 182 passed, 0 failed ``` Local oracle: @@ -81,16 +82,16 @@ Result: oracle: 42 passed, 0 failed ``` -Focused upstream case fixed in this checkpoint: +Focused upstream guardrail containing the cases fixed in this checkpoint: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_exact_sources$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_exception|taint_exact_sources|taint_field_sensitive1|taint_clean_in_try_no_finally|taint_best_fit_sink|taint_assume_safe_booleans|taint_assume_safe_numbers|taint_assume_safe_indexes)$' LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +upstream-sweep: 8 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 8 compared ``` Intended first-220 upstream sweep: @@ -102,11 +103,11 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep MAX_CASES=220 LIST_MISMATCHES=1 M Result: ```text -upstream-sweep: 187 passed, 31 mismatched, 0 jerboa errors, 2 current errors, 220 compared +upstream-sweep: 189 passed, 29 mismatched, 0 jerboa errors, 2 current errors, 220 compared ``` -Full current upstream sweep, accidentally run with `CASE_LIMIT=220` which this -script ignores: +The full 241-case sweep was not rerun after this checkpoint. The last full +run, before the `taint_exception` and `taint_field_sensitive1` fixes, was: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_LIMIT=220 LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh @@ -123,53 +124,36 @@ errors. ## What Changed In This Checkpoint -This checkpoint clears `taint_exact_sources`, which was the next mismatch after -`7beb039`. +This checkpoint clears `taint_exception` and `taint_field_sensitive1`, the next +two mismatches after `29dd690`. Implementation changes in `src/semgrep/scan.ss`: -- Python implicit propagation now includes loop variables with: - - ```scheme - "for $L in $R:\n ..." - ``` - - This models `for res in results:` as propagation from `results` to `res`. - -- Assignment-kill matching is broader and container-aware. A clean assignment - can now kill an earlier source when the assignment LHS contains the source - token, not only when the assignment target binding is exactly equal to the - source text. This is what suppresses stale function-parameter taint after - clean writes like `params["sql"] = "safe"`. - -- Propagator applicability now checks whether an earlier clean assignment has - killed the source before the propagator RHS. This prevents stale container - taint from re-propagating after a clean write. - -- Duplicate implicit assignment matches for the same target statement are - excluded by finding range, not just by object identity. This matters because - `params["sql"] = params[...]` matches both `$L = $R` and `$L[$I] = $R`. - Without the range-based exclusion, one implicit propagator kills the other - while it is trying to inspect the same RHS. - -- `expand-taint-sources` now receives the implicit assignment propagators as - assignment-kill facts, so propagation and sink reachability use the same - clean-assignment information. +- Python taint facts are now filtered out of trivially impossible + `try`/`except`/`else` branches. The filter is deliberately narrow: + `try: pass` makes a matching `except` branch impossible, `try: raise ...` or + `try: return ...` makes the matching `else` branch impossible, and unknown + calls keep both branches possible. +- The reachability filter is applied only to taint source, sanitizer, sink, and + propagator findings. General structural matching is untouched. +- Field/member taint can now reach an opaque base sink call. For example, if + `x.a` is tainted, `sink(x)` is treated as tainted because the callee could + inspect `x.a`, while `sink(x.b)` is not treated as tainted by `x.a`. New smoke coverage in `tests/smoke.ss`: ```text -scan taint indexed assignment can re-taint same base -scan Python taint propagates through loop variables +scan Python taint filters impossible exception branches +scan taint field source reaches opaque base sink ``` -The first test locks the subtle `params["sql"] = "x" % params["test"]` behavior: -a tainted write should re-taint `params`, while a clean indexed write should -not. The second test locks basic `for item in tainted_iterable` propagation. +The exception test covers `try: pass`, `try: raise`, and unknown-call try +bodies. The field-source test covers `x.a` taint reaching `sink(x)` without +also tainting unrelated `sink(x.b)`. -## Resolved Case: taint_exact_sources +## Resolved Recent Cases -Focused command: +`taint_exact_sources` was cleared in `29dd690`: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_exact_sources$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh @@ -205,13 +189,55 @@ Expected matched lines for this case are now exactly: 7, 28, 33, 49, 60, 69 ``` +`taint_exception` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_exception$' LIST_MISMATCHES=1 MAX_DIFFS=220 tests/oracle/upstream-sweep.sh +``` + +Current result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +What changed: + +- Jerboa previously reported all expected lines plus false positives at + 29, 35, 74, 80, 106, 122, and 179. +- Those false positives came from assignments in impossible `except` or `else` + branches. +- The new Python taint branch filter removes only trivial impossible branches: + `try: pass` suppresses `except`, `try: raise`/`return` suppresses `else`, + and `any_function_call_may_raise()` remains unknown. +- Expected `finally` propagation around lines 180 to 196 remains intact. + +`taint_field_sensitive1` is now cleared: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_field_sensitive1$' LIST_MISMATCHES=1 MAX_DIFFS=220 tests/oracle/upstream-sweep.sh +``` + +Current result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +What changed: + +- Jerboa previously missed line 32, `sink(x)`, after `x.a`, `x.c`, and `x.d` + were tainted. +- The new opaque-base compatibility lets a member source such as `x.a` taint + a whole-object sink argument `x`. +- The compatibility is prefix-bounded, so `x.a` does not taint unrelated + `sink(x.b)`. + ## Current First-220 Frontier -The current first-220 sweep has these 31 mismatches: +The current first-220 sweep has these 29 mismatches: ```text -taint_exception -taint_field_sensitive1 taint_field_sensitive2 taint_field_sensitive3 taint_field_sensitive4 @@ -259,12 +285,12 @@ vardef_assign_true1 vardef_assign_true2 ``` -## Immediate Next Case: taint_exception +## Immediate Next Case: taint_field_sensitive2 Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_exception$' LIST_MISMATCHES=1 MAX_DIFFS=200 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_field_sensitive2$' LIST_MISMATCHES=1 MAX_DIFFS=220 tests/oracle/upstream-sweep.sh ``` Current result: @@ -277,82 +303,67 @@ Rule: ```yaml rules: -- id: python-exception +- id: test mode: taint pattern-sources: - - pattern: input - pattern-sanitizers: - - pattern: sanitize(...) + - pattern: source pattern-sinks: - pattern: sink(...) - message: Match found + message: test languages: - - python - severity: ERROR + - typescript + - javascript + severity: WARNING ``` Target: ```text -/Users/user/mine/semgrep/tests/rules/taint_exception.py +/Users/user/mine/semgrep/tests/rules/taint_field_sensitive2.js ``` -Packaged Semgrep expects these sink lines: +Relevant target: ```text -108, 124, 139, 141, 174, 192, 194, 196 +3 x = source +4 x.a = safe +5 x.c[i].d = safe +8 sink(x) +10 sink(x.b) +12 sink(x.b.c) +14 sink(x.c[j]) +17 sink(x.a) +19 sink(x.a.b) +22 sink(x.c[j].d) +24 sink(x.c[j].d.e) ``` -Jerboa currently reports all expected lines plus false positives at: +Packaged Semgrep expects findings on: ```text -29, 35, 74, 80, 106, 122, 179 +8, 10, 12, 14 ``` -Normalized diff summary: +Jerboa currently reports no findings: ```diff -@@ -1,8 +1,15 @@ -+line 106 sink(clean) - line 108 sink(dirty) -+line 122 sink(clean) - line 124 sink(dirty) - line 139 sink(dirty1) - line 141 sink(dirty2) - line 174 sink(clean1) -+line 179 sink(clean2) - line 192 sink(dirty1) - line 194 sink(dirty2) - line 196 sink(dirty3) -+line 29 sink(clean) -+line 35 sink(clean) -+line 74 sink(clean) -+line 80 sink(clean) +@@ -1,4 +0,0 @@ +-(finding "test" ".../taint_field_sensitive2.js" 10 5 ... "WARNING" "test" "") +-(finding "test" ".../taint_field_sensitive2.js" 12 5 ... "WARNING" "test" "") +-(finding "test" ".../taint_field_sensitive2.js" 14 5 ... "WARNING" "test" "") +-(finding "test" ".../taint_field_sensitive2.js" 8 5 ... "WARNING" "test" "") ``` Interpretation for the next fix: -- This is path-sensitive Python exception reachability, not import matching or - basic assignment propagation. -- The target intentionally includes unreachable sinks after unconditional - `raise` and `return`, and impossible `except` or `else` paths when the `try` - body is trivially known to raise or not raise. -- Current Jerboa taint is mostly path-insensitive across Python `try`, - `except`, `else`, and `finally` blocks. It propagates assignments that - packaged Semgrep considers unreachable for this fixture. -- Be careful around `finally`: packaged Semgrep expects finally assignments to - remain reachable in the nested case around lines 180 to 196. - -Likely next implementation area: - -- Add a narrow Python reachability guard for taint source/assignment matches - that are after unconditional `raise` or `return` in the same simple block. -- Add enough try/except/else/finally reasoning for trivial cases: - `try: raise ...` should make `except` reachable and `else` unreachable; - `try: pass` should make `else` reachable and `except` unreachable; - calls like `any_function_call_may_raise()` should keep both paths possible. -- Preserve existing passing guardrails, especially - `taint_clean_in_try_no_finally` and the first-160 taint cluster. +- This is the inverse of `taint_field_sensitive1`: a whole-object source + (`x = source`) should taint member sinks such as `x.b` and `x.c[j]`. +- Clean member assignments should suppress only their matching field subtree: + `x.a = safe` suppresses `x.a` and `x.a.b`; `x.c[i].d = safe` suppresses + `x.c[j].d` and `x.c[j].d.e` but still allows `x.c[j]`. +- Likely implementation area is field-prefix compatibility plus field-specific + assignment kills. Be careful not to undo the `taint_field_sensitive1` + behavior where member taint reaches an opaque whole-object sink. ## Resolved Earlier Frontier --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -7768,6 +7768,166 @@ (taint-function-parameter-source-spec? spec) (taint-bare-identifier-source-spec? spec)) #f (alist-ref/default spec 'control #f)))) + (def (python-next-line-start source line-start) + (let* ([line-end (line-end-after source line-start)] + [len (string-length source)]) + (and (< line-end len) (+ line-end 1)))) + (def (python-previous-line-start source line-start) + (and (> line-start 0) + (line-start-before source (- line-start 1)))) + (def (python-significant-trimmed-line source line-start) + (let* ([line-end (line-end-after source line-start)] + [trimmed (string-trim + (substring source line-start line-end))]) + (and (> (string-length trimmed) 0) + (not (sg-string-prefix? "#" trimmed)) + trimmed))) + (def (python-clause-kind trimmed) + (cond + [(or (sg-string-prefix? "except " trimmed) + (sg-string-prefix? "except:" trimmed)) + 'except] + [(sg-string-prefix? "else:" trimmed) 'else] + [(sg-string-prefix? "finally:" trimmed) 'finally] + [(sg-string-prefix? "try:" trimmed) 'try] + [else #f])) + (def (python-parent-exception-clause source offset) + (let* ([line-start (line-start-before source offset)] + [target-indent (line-indent-at-offset source offset)]) + (let loop ([current-start line-start] + [current-indent target-indent]) + (let ([prev-start (python-previous-line-start + source + current-start)]) + (if (not prev-start) + #f + (let ([trimmed (python-significant-trimmed-line + source + prev-start)]) + (if (not trimmed) + (loop prev-start current-indent) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (if (>= indent current-indent) + (loop prev-start current-indent) + (let ([kind (python-clause-kind trimmed)]) + (if (and kind + (or (eq? kind 'except) + (eq? kind 'else) + (eq? kind 'finally))) + (list + (cons 'kind kind) + (cons 'indent indent) + (cons 'start prev-start)) + (loop prev-start indent)))))))))))) + (def (python-matching-try-line source clause) + (let ([clause-start (alist-ref/default clause 'start #f)] + [clause-indent (alist-ref/default clause 'indent 0)]) + (and clause-start + (let loop ([current-start clause-start]) + (let ([prev-start (python-previous-line-start + source + current-start)]) + (if (not prev-start) + #f + (let ([trimmed (python-significant-trimmed-line + source + prev-start)]) + (if (not trimmed) + (loop prev-start) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (cond + [(< indent clause-indent) #f] + [(> indent clause-indent) + (loop prev-start)] + [(eq? (python-clause-kind trimmed) 'try) + prev-start] + [else (loop prev-start)])))))))))) + (def (python-pass-line? trimmed) + (or (string=? trimmed "pass") + (sg-string-prefix? "pass #" trimmed))) + (def (python-exit-line? trimmed) + (or (string=? trimmed "raise") + (sg-string-prefix? "raise " trimmed) + (string=? trimmed "return") + (sg-string-prefix? "return " trimmed))) + (def (python-try-body-only-pass? + source + line-start + try-indent + body-indent) + (let loop ([current-start line-start]) + (or (not current-start) + (let ([trimmed (python-significant-trimmed-line + source + current-start)]) + (if (not trimmed) + (loop (python-next-line-start source current-start)) + (let ([indent (line-indent-at-offset + source + current-start)]) + (cond + [(<= indent try-indent) #t] + [(not (= indent body-indent)) #f] + [(python-pass-line? trimmed) + (loop + (python-next-line-start source current-start))] + [else #f]))))))) + (def (python-try-body-outcome source try-start) + (let* ([try-indent (line-indent-at-offset source try-start)] + [first-body-line (python-next-line-start source try-start)]) + (let loop ([current-start first-body-line]) + (if (not current-start) + 'unknown + (let ([trimmed (python-significant-trimmed-line + source + current-start)]) + (if (not trimmed) + (loop (python-next-line-start source current-start)) + (let ([indent (line-indent-at-offset + source + current-start)]) + (cond + [(<= indent try-indent) 'unknown] + [(python-exit-line? trimmed) 'exits] + [(and (python-pass-line? trimmed) + (python-try-body-only-pass? + source + (python-next-line-start + source + current-start) + try-indent + indent)) + 'no-throw] + [else 'unknown])))))))) + (def (python-finding-in-impossible-exception-branch? + source + finding) + (let* ([clause (python-parent-exception-clause + source + (finding-start-offset finding))] + [kind (and clause (alist-ref/default clause 'kind #f))] + [try-start (and clause + (python-matching-try-line source clause))] + [outcome (and try-start + (python-try-body-outcome source try-start))]) + (or (and (eq? kind 'except) (eq? outcome 'no-throw)) + (and (eq? kind 'else) (eq? outcome 'exits))))) + (def (filter-python-taint-reachable-findings + language + source + findings) + (if (symbolic-python-like-language? language) + (sg-filter + (lambda (finding) + (not (python-finding-in-impossible-exception-branch? + source + finding))) + findings) + findings)) (def (scan-taint-specs rule specs language path source target-root default-label) (apply @@ -7775,8 +7935,11 @@ (map (lambda (spec) (map (lambda (finding) (taint-state-for-spec spec finding default-label)) - (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language - path source target-root))) + (filter-python-taint-reachable-findings + language + source + (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language + path source target-root)))) specs))) (def (scan-taint-source-matches rule specs language path source target-root) @@ -7794,8 +7957,11 @@ (cons 'requires (alist-ref/default spec 'requires #f)))) - (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language - path source target-root))) + (filter-python-taint-reachable-findings + language + source + (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language + path source target-root)))) specs))) (def (source-match-state match) (alist-ref/default match 'state #f)) @@ -7816,8 +7982,11 @@ (cons 'requires (alist-ref/default spec 'requires #f)))) - (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language - path source target-root))) + (filter-python-taint-reachable-findings + language + source + (scan-positive-pattern-entry rule (alist-ref/default spec 'entry #f) language + path source target-root)))) specs))) (def (scan-taint-propagators rule propagators language path source target-root) @@ -7848,8 +8017,11 @@ propagator 'replace-labels #f)))) - (scan-positive-pattern-entry rule (alist-ref/default propagator 'entry #f) - language path source target-root))) + (filter-python-taint-reachable-findings + language + source + (scan-positive-pattern-entry rule (alist-ref/default propagator 'entry #f) + language path source target-root)))) propagators))) (def (implicit-assignment-patterns language) (cond @@ -7879,8 +8051,11 @@ (cons 'to "$L") (cons 'by-side-effect #t) (cons 'label #f) (cons 'requires #f) (cons 'replace-labels #f))) - (scan-positive-pattern-entry rule (cons 'pattern pattern) language path source - target-root))) + (filter-python-taint-reachable-findings + language + source + (scan-positive-pattern-entry rule (cons 'pattern pattern) language path source + target-root)))) (implicit-assignment-patterns language)))) (def (label-token-char? ch) (or (char-alphabetic? ch) @@ -8014,6 +8189,42 @@ (lambda (entry) (binding-token-in-finding? (cdr entry) sink source-text)) (finding-metavars source)))) + (def (field-source-taints-base-text? source-text base-text) + (let ([base-len (string-length base-text)] + [source-len (string-length source-text)]) + (and (> base-len 0) + (> source-len base-len) + (substring-at? source-text base-text 0) + (let ([next (string-ref source-text base-len)]) + (or (char=? next #\.) (char=? next #\[)))))) + (def (direct-call-argument-text sink source-text) + (let* ([sink-text (finding-text sink source-text)] + [open (string-find-substring sink-text "(")] + [close (and open + (> (string-length sink-text) 0) + (let ([last (- (string-length sink-text) 1)]) + (and (char=? (string-ref sink-text last) #\)) + last)))]) + (and open + close + (string-trim (substring sink-text (+ open 1) close))))) + (def (field-source-taints-base-sink? + source + sink + source-text) + (let ([arg-text (direct-call-argument-text + sink + source-text)]) + (and arg-text + (or (field-source-taints-base-text? + (finding-text source source-text) + arg-text) + (any? + (lambda (entry) + (field-source-taints-base-text? + (metavariable-binding-text (cdr entry)) + arg-text)) + (finding-metavars source)))))) (def (line-indent line) (let ([len (string-length line)]) (let loop ([i 0]) @@ -8070,6 +8281,10 @@ source sink source-text)) + (field-source-taints-base-sink? + source + sink + source-text) (and (null? (finding-metavars source)) (null? (finding-metavars sink)) (finding-range-contains? sink source)))))))) --- 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" . "BDFF8384FA069AE1") - ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") + ("src/semgrep/scan.ss" . "38CD0141D5589AF0") ("src/semgrep/rule.ss" . "E12C108153C181FA") - ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") + ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("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 @@ -8266,18 +8266,163 @@ #f (alist-ref/default spec 'control #f)))) +(def (python-next-line-start source line-start) + (let* ([line-end (line-end-after source line-start)] + [len (string-length source)]) + (and (< line-end len) + (+ line-end 1)))) + +(def (python-previous-line-start source line-start) + (and (> line-start 0) + (line-start-before source (- line-start 1)))) + +(def (python-significant-trimmed-line source line-start) + (let* ([line-end (line-end-after source line-start)] + [trimmed (string-trim (substring source line-start line-end))]) + (and (> (string-length trimmed) 0) + (not (sg-string-prefix? "#" trimmed)) + trimmed))) + +(def (python-clause-kind trimmed) + (cond + [(or (sg-string-prefix? "except " trimmed) + (sg-string-prefix? "except:" trimmed)) + 'except] + [(sg-string-prefix? "else:" trimmed) 'else] + [(sg-string-prefix? "finally:" trimmed) 'finally] + [(sg-string-prefix? "try:" trimmed) 'try] + [else #f])) + +(def (python-parent-exception-clause source offset) + (let* ([line-start (line-start-before source offset)] + [target-indent (line-indent-at-offset source offset)]) + (let loop ([current-start line-start] [current-indent target-indent]) + (let ([prev-start (python-previous-line-start source current-start)]) + (if (not prev-start) + #f + (let ([trimmed (python-significant-trimmed-line source prev-start)]) + (if (not trimmed) + (loop prev-start current-indent) + (let ([indent (line-indent-at-offset source prev-start)]) + (if (>= indent current-indent) + (loop prev-start current-indent) + (let ([kind (python-clause-kind trimmed)]) + (if (and kind + (or (eq? kind 'except) + (eq? kind 'else) + (eq? kind 'finally))) + (list (cons 'kind kind) + (cons 'indent indent) + (cons 'start prev-start)) + (loop prev-start indent)))))))))))) + +(def (python-matching-try-line source clause) + (let ([clause-start (alist-ref/default clause 'start #f)] + [clause-indent (alist-ref/default clause 'indent 0)]) + (and clause-start + (let loop ([current-start clause-start]) + (let ([prev-start (python-previous-line-start source current-start)]) + (if (not prev-start) + #f + (let ([trimmed (python-significant-trimmed-line + source + prev-start)]) + (if (not trimmed) + (loop prev-start) + (let ([indent (line-indent-at-offset + source + prev-start)]) + (cond + [(< indent clause-indent) #f] + [(> indent clause-indent) (loop prev-start)] + [(eq? (python-clause-kind trimmed) 'try) prev-start] + [else (loop prev-start)])))))))))) + +(def (python-pass-line? trimmed) + (or (string=? trimmed "pass") + (sg-string-prefix? "pass #" trimmed))) + +(def (python-exit-line? trimmed) + (or (string=? trimmed "raise") + (sg-string-prefix? "raise " trimmed) + (string=? trimmed "return") + (sg-string-prefix? "return " trimmed))) + +(def (python-try-body-only-pass? source line-start try-indent body-indent) + (let loop ([current-start line-start]) + (or (not current-start) + (let ([trimmed (python-significant-trimmed-line + source + current-start)]) + (if (not trimmed) + (loop (python-next-line-start source current-start)) + (let ([indent (line-indent-at-offset source current-start)]) + (cond + [(<= indent try-indent) #t] + [(not (= indent body-indent)) #f] + [(python-pass-line? trimmed) + (loop (python-next-line-start source current-start))] + [else #f]))))))) + +(def (python-try-body-outcome source try-start) + (let* ([try-indent (line-indent-at-offset source try-start)] + [first-body-line (python-next-line-start source try-start)]) + (let loop ([current-start first-body-line]) + (if (not current-start) + 'unknown + (let ([trimmed (python-significant-trimmed-line source current-start)]) + (if (not trimmed) + (loop (python-next-line-start source current-start)) + (let ([indent (line-indent-at-offset source current-start)]) + (cond + [(<= indent try-indent) 'unknown] + [(python-exit-line? trimmed) 'exits] + [(and (python-pass-line? trimmed) + (python-try-body-only-pass? + source + (python-next-line-start source current-start) + try-indent + indent)) + 'no-throw] + [else 'unknown])))))))) + +(def (python-finding-in-impossible-exception-branch? source finding) + (let* ([clause (python-parent-exception-clause + source + (finding-start-offset finding))] + [kind (and clause (alist-ref/default clause 'kind #f))] + [try-start (and clause (python-matching-try-line source clause))] + [outcome (and try-start (python-try-body-outcome source try-start))]) + (or (and (eq? kind 'except) + (eq? outcome 'no-throw)) + (and (eq? kind 'else) + (eq? outcome 'exits))))) + +(def (filter-python-taint-reachable-findings language source findings) + (if (symbolic-python-like-language? language) + (sg-filter + (lambda (finding) + (not (python-finding-in-impossible-exception-branch? + source + finding))) + findings) + findings)) + (def (scan-taint-specs rule specs language path source target-root default-label) (apply append (map (lambda (spec) (map (lambda (finding) (taint-state-for-spec spec finding default-label)) - (scan-positive-pattern-entry - rule - (alist-ref/default spec 'entry #f) + (filter-python-taint-reachable-findings language - path source - target-root))) + (scan-positive-pattern-entry + rule + (alist-ref/default spec 'entry #f) + language + path + source + target-root)))) specs))) (def (scan-taint-source-matches rule specs language path source target-root) @@ -8292,13 +8437,16 @@ "__SOURCE__")) (cons 'requires (alist-ref/default spec 'requires #f)))) - (scan-positive-pattern-entry - rule - (alist-ref/default spec 'entry #f) + (filter-python-taint-reachable-findings language - path source - target-root))) + (scan-positive-pattern-entry + rule + (alist-ref/default spec 'entry #f) + language + path + source + target-root)))) specs))) (def (source-match-state match) @@ -8324,13 +8472,16 @@ #f)))) (cons 'requires (alist-ref/default spec 'requires #f)))) - (scan-positive-pattern-entry - rule - (alist-ref/default spec 'entry #f) + (filter-python-taint-reachable-findings language - path source - target-root))) + (scan-positive-pattern-entry + rule + (alist-ref/default spec 'entry #f) + language + path + source + target-root)))) specs))) (def (scan-taint-propagators rule propagators language path source target-root) @@ -8354,13 +8505,16 @@ (alist-ref/default propagator 'replace-labels #f)))) - (scan-positive-pattern-entry - rule - (alist-ref/default propagator 'entry #f) + (filter-python-taint-reachable-findings language - path source - target-root))) + (scan-positive-pattern-entry + rule + (alist-ref/default propagator 'entry #f) + language + path + source + target-root)))) propagators))) (def (implicit-assignment-patterns language) @@ -8404,13 +8558,16 @@ (cons 'label #f) (cons 'requires #f) (cons 'replace-labels #f))) - (scan-positive-pattern-entry - rule - (cons 'pattern pattern) + (filter-python-taint-reachable-findings language - path source - target-root))) + (scan-positive-pattern-entry + rule + (cons 'pattern pattern) + language + path + source + target-root)))) (implicit-assignment-patterns language)))) (def (label-token-char? ch) @@ -8547,6 +8704,40 @@ (binding-token-in-finding? (cdr entry) sink source-text)) (finding-metavars source)))) +(def (field-source-taints-base-text? source-text base-text) + (let ([base-len (string-length base-text)] + [source-len (string-length source-text)]) + (and (> base-len 0) + (> source-len base-len) + (substring-at? source-text base-text 0) + (let ([next (string-ref source-text base-len)]) + (or (char=? next #\.) + (char=? next #\[)))))) + +(def (direct-call-argument-text sink source-text) + (let* ([sink-text (finding-text sink source-text)] + [open (string-find-substring sink-text "(")] + [close (and open + (> (string-length sink-text) 0) + (let ([last (- (string-length sink-text) 1)]) + (and (char=? (string-ref sink-text last) #\)) + last)))]) + (and open + close + (string-trim (substring sink-text (+ open 1) close))))) + +(def (field-source-taints-base-sink? source sink source-text) + (let ([arg-text (direct-call-argument-text sink source-text)]) + (and arg-text + (or (field-source-taints-base-text? + (finding-text source source-text) + arg-text) + (any? (lambda (entry) + (field-source-taints-base-text? + (metavariable-binding-text (cdr entry)) + arg-text)) + (finding-metavars source)))))) + (def (line-indent line) (let ([len (string-length line)]) (let loop ([i 0]) @@ -8591,11 +8782,15 @@ (finding-range-equal? source sink) (or (finding-range-contains? sink source) (and (taint-state-contained? source-state) - (finding-range-contains? sink source)) + (finding-range-contains? sink source)) (finding-text-equals-any-binding? source sink source-text) (finding-text-equals-any-binding? sink source source-text) (and (taint-state-token? source-state) (source-token-in-sink? source sink source-text)) + (field-source-taints-base-sink? + source + sink + source-text) (and (null? (finding-metavars source)) (null? (finding-metavars sink)) (finding-range-contains? sink source)))))))) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1593,6 +1593,34 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 3))) +(test-case "scan Python taint filters impossible exception branches" + (let* ([taint-config + "rules:\n - id: demo.taint.exception.paths\n mode: taint\n languages: [python]\n message: exception path taint\n severity: WARNING\n pattern-sources:\n - pattern: input\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "python" + "demo.py" + "def no_throw(input):\n clean = None\n dirty = None\n try:\n pass\n except Exception:\n clean = input\n else:\n dirty = input\n sink(clean)\n sink(dirty)\n\ndef always_raises(input):\n clean = None\n dirty = None\n try:\n raise RuntimeError()\n except Exception:\n dirty = input\n else:\n clean = input\n sink(clean)\n sink(dirty)\n\ndef maybe_raises(input):\n left = None\n right = None\n try:\n may_raise()\n except Exception:\n left = input\n else:\n right = input\n sink(left)\n sink(right)\n")]) + (check (length findings) => 4) + (check (finding-start-line (car findings)) => 11) + (check (finding-start-line (cadr findings)) => 23) + (check (finding-start-line (caddr findings)) => 34) + (check (finding-start-line (cadddr findings)) => 35))) + +(test-case "scan taint field source reaches opaque base sink" + (let* ([taint-config + "rules:\n - id: demo.taint.field-base\n mode: taint\n languages: [javascript]\n message: field base taint\n severity: WARNING\n pattern-sources:\n - pattern: source\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function f() {\n x.a = source;\n x.b = safe;\n sink(x.a);\n sink(x);\n sink(x.b);\n}\n")]) + (check (length findings) => 2) + (check (finding-start-line (car findings)) => 4) + (check (finding-start-line (cadr findings)) => 5))) + (test-case "scan taint by-side-effect source reaches wildcard sink" (let* ([taint-config "rules:\n - id: demo.taint.by-side-effect-source\n mode: taint\n languages: [python]\n message: side-effect taint\n severity: WARNING\n pattern-sources:\n - by-side-effect: true\n patterns:\n - pattern: $X = source()\n - focus-metavariable: $X\n pattern-sinks:\n - pattern: sink(...)\n"]