Advance taint safe function parity
ober
e429d340b8e30ee37b568b1f2b8863d07060c715
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -65,7 +65,7 @@ make test Result: ```text -174 tests, 174 passed, 0 failed +176 tests, 176 passed, 0 failed ``` Local oracle: @@ -80,16 +80,28 @@ Result: oracle: 42 passed, 0 failed ``` -Focused upstream case fixed in this checkpoint: +Focused upstream cases fixed in this checkpoint: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_best_fit_sink2$' LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_best_fit_sink3|taint_clean_in_try_no_finally)$' 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: 2 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 2 compared +``` + +Focused safe-option/propagation guardrail: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_best_fit_sink|taint_best_fit_sink2|taint_best_fit_sink3|taint_clean_in_try_no_finally|taint_assume_safe_indexes|taint_assume_safe_numbers|taint_assume_safe_booleans)$' LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 7 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 7 compared ``` First 220 upstream rule/target pairs: @@ -101,25 +113,27 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep MAX_CASES=220 LIST_MISMATCHES=1 M Result: ```text -upstream-sweep: 181 passed, 37 mismatched, 0 jerboa errors, 2 current errors, 220 compared +upstream-sweep: 183 passed, 35 mismatched, 0 jerboa errors, 2 current errors, 220 compared ``` Important progress markers: - The first 160 upstream pairs are still clean for Jerboa: 158 passed, 0 mismatched, 0 Jerboa errors, 2 packaged-Semgrep current errors. -- The first-220 sweep improved from 38 mismatches to 37 mismatches in this +- The first-220 sweep improved from 37 mismatches to 35 mismatches in this checkpoint. -- `taint_best_fit_sink2` is no longer part of the mismatch frontier. +- `taint_best_fit_sink3` and `taint_clean_in_try_no_finally` are no longer part + of the mismatch frontier. - The 2 non-passing current errors in the first-160/first-220 windows are packaged-Semgrep oracle errors, not Jerboa scanner errors. ## What Changed In This Checkpoint -The current checkpoint is a taint best-fit/sanitizer correction for Python -conditional expressions and nested sanitizer calls. The source changes are in -`src/semgrep/scan.ss`; `make test` mirrored them into -`lib/semgrep/scan.sls`. +The current checkpoint extends the taint best-fit/sanitizer work. It adds +support for `taint_assume_safe_functions` in Python focused sink shapes and +prevents later implicit assignments from resurrecting token taint after a +compatible sanitizer has run. The source changes are in `src/semgrep/scan.ss`; +`make test` mirrored them into `lib/semgrep/scan.sls`. New implementation landmarks: @@ -130,6 +144,12 @@ finding-range-contains-span? sanitizer-covers-token-span? token-text-fully-sanitized-in-sink? token-source-fully-sanitized-in-sink? +taint-assume-safe-functions? +call-name-start-before-open +matching-close-paren-index +finding-focused-to-whole-binding? +safe-function-wrapper-in-text? +taint-safe-function-use? ``` Behavior added: @@ -148,6 +168,15 @@ Behavior added: token source cannot taint an earlier sink merely because the text matches. Token sources now still need to be before the sink or physically contained in it when the non-exact sink branch is used. +- With `taint_assume_safe_functions: true`, taint nested under an ordinary + function result is suppressed, e.g. `sink(not_a_propagator(tainted()))`, + while direct sink arguments such as `sink(tainted())` still report. +- The safe-function check handles focused sink findings by recognizing when the + finding range is exactly the focused metavariable binding. +- Propagation through later implicit assignments is blocked when the source was + sanitized before the assignment RHS, so `id = int(id)` prevents later + `some_object = id` / `csv_file = ...` propagation from reviving the old + `request` taint. Existing sanitizer logic changed: @@ -159,6 +188,8 @@ Smoke coverage added: ```text scan taint sanitizer blocks conditional sink token +scan taint assume safe functions blocks wrapper +scan taint sanitizer blocks later assignment propagation ``` That smoke test mirrors upstream `taint_best_fit_sink2.py`: @@ -174,11 +205,9 @@ Expected result: zero findings. ## Current Upstream Frontier -The first-220 sweep currently has these 37 Jerboa mismatches: +The first-220 sweep currently has these 35 Jerboa mismatches: ```text -taint_best_fit_sink3 -taint_clean_in_try_no_finally taint_control taint_exact_sources taint_exception @@ -216,20 +245,21 @@ taint_react taint_safe_comparisons ``` -The next case to investigate is `taint_best_fit_sink3`. +The next case to investigate is `taint_control`. Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_best_fit_sink3$' LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_control$' LIST_MISMATCHES=1 MAX_DIFFS=200 tests/oracle/upstream-sweep.sh ``` Current normalized diff: ```diff -@@ -1 +1,2 @@ -+(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_best_fit_sink3.py" 11 6 133 11 33 160 "WARNING" "Match" "") - (finding "test" "/Users/user/mine/semgrep/tests/rules/taint_best_fit_sink3.py" 2 6 19 2 15 28 "WARNING" "Match" "") +@@ -1,3 +0,0 @@ +-(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_control.py" 14 5 140 14 11 146 "WARNING" "Test" "") +-(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_control.py" 22 5 228 22 11 234 "WARNING" "Test" "") +-(finding "test" "/Users/user/mine/semgrep/tests/rules/taint_control.py" 6 3 58 6 9 64 "WARNING" "Test" "") ``` Rule: @@ -239,51 +269,52 @@ rules: - id: test languages: - python - message: Match + severity: WARNING mode: taint - options: - taint_assume_safe_functions: true - taint_assume_safe_indexes: true - pattern-sinks: - - patterns: - - pattern: sink($X) - - focus-metavariable: $X + message: Test pattern-sources: - - pattern: tainted(...) - severity: WARNING + - control: true + pattern: source(...) + pattern-sinks: + - pattern: sink(...) ``` Target: ```python -#ruleid: test -sink(tainted()) - -# ok:test -sink(ok1 if tainted() else ok2) - -# ok:test -sink([ok1 if tainted() else ok2]) - -#ok:test -sink(not_a_propagator(tainted())) - -#ok:test -sink(some_array[tainted()]) +def test1(): + source() + foo() + bar() + #ruleid: test + sink() + +def test2(): + source() + foo() + bar() + if baz(): + #ruleid: test + sink() + +def test3(): + if foo(): + source() + bar() + if baz(): + #ruleid: test + sink() ``` Interpretation for the next fix: -- Jerboa correctly reports line 2. -- Jerboa incorrectly reports line 11, whose focused sink `$X` is - `some_array[tainted()]`. -- The rule has `taint_assume_safe_indexes: true`, so a source used only as an - index should be considered safe. -- There is already code around `taint-safe-index-use?`, - `finding-starts-inside-square-brackets?`, and - `taint-source-token-inside-square-brackets?`; the next patch should likely - make this logic work for focused sink findings whose range is the indexed - expression rather than the full `sink(...)` call. +- Jerboa currently reports zero findings for control-taint sources. +- Semgrep reports all three sinks after a `control: true` source in the same + function/control region. +- The parser already records `control` on taint source specs; the next patch + should inspect `taint-state-for-spec`, source reachability, and label + propagation to model control taint without making ordinary value taint leak + across unrelated functions. ## Resolved First-160 Taint Cluster @@ -336,6 +367,8 @@ scan JavaScript taint object destructuring assignment scan TypeScript taint source after trailing pattern-inside scan taint indexed assignment taints and clears base scan taint sanitizer blocks conditional sink token +scan taint assume safe functions blocks wrapper +scan taint sanitizer blocks later assignment propagation ``` ## Earlier Parity Work Worth Preserving @@ -451,9 +484,8 @@ Use the target extension that actually exists for the case (`.py`, `.js`, ## Immediate Next Step -Start with `taint_best_fit_sink3`. Add a smoke test that expects only -`sink(tainted())` to report when `taint_assume_safe_indexes: true` and the sink -is focused to `$X`. Then patch the safe-index logic so -`sink(some_array[tainted()])` is suppressed without regressing the existing -`scan taint non-exact sink includes nested source` and -`scan taint indexed assignment taints and clears base` smoke tests. +Start with `taint_control`. Add a smoke test for `control: true` sources where +`source()` causes later `sink()` calls in the same function to report, including +inside a later `if` block. Then patch taint reachability so control-taint flows +through control/order within the same simple function scope without changing +ordinary value-taint behavior. --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -7997,6 +7997,8 @@ (rule-option-enabled? rule "taint_assume_safe_numbers")) (def (taint-assume-safe-indexes? rule) (rule-option-enabled? rule "taint_assume_safe_indexes")) + (def (taint-assume-safe-functions? rule) + (rule-option-enabled? rule "taint_assume_safe_functions")) (def (text-contains-comparison? text) (or (string-find-substring text "==") (string-find-substring text "!=") @@ -8120,6 +8122,106 @@ (string=? (string-trim (substring sink-text (+ open 1) close)) (string-trim source-text0))))) + (def (call-name-start-before-open text open-index) + (let loop ([i (- open-index 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) (loop (- i 1))] + [(identifier-token-char? (string-ref text i)) + (let name-loop ([j i]) + (if (and (> j 0) + (identifier-token-char? (string-ref text (- j 1)))) + (name-loop (- j 1)) + j))] + [else #f]))) + (def (matching-close-paren-index text open-index) + (let ([len (string-length text)]) + (let loop ([i (+ open-index 1)] + [depth 1] + [state 'normal] + [escaped? #f]) + (cond + [(>= i len) #f] + [(eq? state 'string) + (let ([ch (string-ref text i)]) + (cond + [escaped? (loop (+ i 1) depth state #f)] + [(char=? ch #\\) (loop (+ i 1) depth state #t)] + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) depth 'normal #f)] + [else (loop (+ i 1) depth state #f)]))] + [else + (let ([ch (string-ref text i)]) + (cond + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) depth 'string #f)] + [(char=? ch #\() (loop (+ i 1) (+ depth 1) state #f)] + [(char=? ch #\)) + (if (= depth 1) i (loop (+ i 1) (- depth 1) state #f))] + [else (loop (+ i 1) depth state #f)]))])))) + (def (finding-focused-to-whole-binding? finding) + (any? + (lambda (entry) + (let ([binding (cdr entry)]) + (and (= (finding-start-offset finding) + (metavariable-binding-start-byte binding)) + (= (finding-end-offset finding) + (metavariable-binding-end-byte binding))))) + (finding-metavars finding))) + (def (safe-function-wrapper-in-text? + text + source-start + source-end + focused?) + (let ([len (string-length text)]) + (let loop ([i 0] [state 'normal] [escaped? #f]) + (cond + [(>= i source-start) #f] + [(eq? state 'string) + (let ([ch (string-ref text i)]) + (cond + [escaped? (loop (+ i 1) state #f)] + [(char=? ch #\\) (loop (+ i 1) state #t)] + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) 'normal #f)] + [else (loop (+ i 1) state #f)]))] + [else + (let ([ch (string-ref text i)]) + (cond + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) 'string #f)] + [(char=? ch #\() + (let ([call-start (call-name-start-before-open text i)] + [close (matching-close-paren-index text i)]) + (or (and call-start + close + (> source-start i) + (<= source-end close) + (not (and (= call-start source-start) + (= (+ close 1) source-end))) + (or focused? + (> call-start 0) + (< (+ close 1) len))) + (loop (+ i 1) state #f)))] + [else (loop (+ i 1) state #f)]))])))) + (def (taint-safe-function-use? + rule + source-state + container + source-text) + (and (taint-assume-safe-functions? rule) + (let ([source (taint-state-finding source-state)]) + (and source + container + (finding-range-contains? container source) + (not (finding-range-equal? container source)) + (let ([container-start (finding-start-offset + container)]) + (safe-function-wrapper-in-text? + (finding-text container source-text) + (- (finding-start-offset source) container-start) + (- (finding-end-offset source) container-start) + (finding-focused-to-whole-binding? container))))))) (def (token-source-inside-function-value-sink? source-state sink @@ -8396,6 +8498,11 @@ (not (and (taint-assume-safe-booleans? rule) (text-contains-comparison? (finding-text sink source-text)))) + (not (taint-safe-function-use? + rule + source-state + sink + source-text)) (not (taint-safe-index-use? rule source-state @@ -8542,11 +8649,30 @@ (let ([sanitizer (taint-state-finding sanitizer-state)]) (and sanitizer from-finding - (finding-range-contains? from-finding sanitizer) - (source-state-reaches-finding? - source-state - sanitizer - source)))) + (or (and (finding-range-contains? from-finding sanitizer) + (source-state-reaches-finding? + source-state + sanitizer + source)) + (let ([source-finding (taint-state-finding + source-state)]) + (and source-finding + (same-simple-function-scope? + source + source-finding + sanitizer) + (same-simple-function-scope? + source + sanitizer + from-finding) + (finding-between? + source-finding + sanitizer + from-finding) + (source-compatible-with-sink? + source-state + sanitizer + source))))))) sanitizers)) (def (taint-propagator-blocked-by-safe-option? rule @@ -8562,6 +8688,11 @@ (and (taint-assume-safe-numbers? rule) from-text (text-contains-numeric-arithmetic? from-text)) + (taint-safe-function-use? + rule + source-state + from-finding + source) (taint-safe-index-use? rule source-state --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,7 +3,7 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "7E5441BD00A7F1D4") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "9851213EC09FA148") + ("src/semgrep/scan.ss" . "E1CA04F548FE150") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/rule.ss" . "E12C108153C181FA") --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -8499,6 +8499,9 @@ (def (taint-assume-safe-indexes? rule) (rule-option-enabled? rule "taint_assume_safe_indexes")) +(def (taint-assume-safe-functions? rule) + (rule-option-enabled? rule "taint_assume_safe_functions")) + (def (text-contains-comparison? text) (or (string-find-substring text "==") (string-find-substring text "!=") @@ -8616,6 +8619,105 @@ (substring sink-text (+ open 1) close)) (string-trim source-text0))))) +(def (call-name-start-before-open text open-index) + (let loop ([i (- open-index 1)]) + (cond + [(< i 0) #f] + [(char-whitespace? (string-ref text i)) + (loop (- i 1))] + [(identifier-token-char? (string-ref text i)) + (let name-loop ([j i]) + (if (and (> j 0) + (identifier-token-char? (string-ref text (- j 1)))) + (name-loop (- j 1)) + j))] + [else #f]))) + +(def (matching-close-paren-index text open-index) + (let ([len (string-length text)]) + (let loop ([i (+ open-index 1)] + [depth 1] + [state 'normal] + [escaped? #f]) + (cond + [(>= i len) #f] + [(eq? state 'string) + (let ([ch (string-ref text i)]) + (cond + [escaped? (loop (+ i 1) depth state #f)] + [(char=? ch #\\) (loop (+ i 1) depth state #t)] + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) depth 'normal #f)] + [else (loop (+ i 1) depth state #f)]))] + [else + (let ([ch (string-ref text i)]) + (cond + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) depth 'string #f)] + [(char=? ch #\() + (loop (+ i 1) (+ depth 1) state #f)] + [(char=? ch #\)) + (if (= depth 1) + i + (loop (+ i 1) (- depth 1) state #f))] + [else (loop (+ i 1) depth state #f)]))])))) + +(def (finding-focused-to-whole-binding? finding) + (any? (lambda (entry) + (let ([binding (cdr entry)]) + (and (= (finding-start-offset finding) + (metavariable-binding-start-byte binding)) + (= (finding-end-offset finding) + (metavariable-binding-end-byte binding))))) + (finding-metavars finding))) + +(def (safe-function-wrapper-in-text? text source-start source-end focused?) + (let ([len (string-length text)]) + (let loop ([i 0] [state 'normal] [escaped? #f]) + (cond + [(>= i source-start) #f] + [(eq? state 'string) + (let ([ch (string-ref text i)]) + (cond + [escaped? (loop (+ i 1) state #f)] + [(char=? ch #\\) (loop (+ i 1) state #t)] + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) 'normal #f)] + [else (loop (+ i 1) state #f)]))] + [else + (let ([ch (string-ref text i)]) + (cond + [(or (char=? ch #\") (char=? ch #\')) + (loop (+ i 1) 'string #f)] + [(char=? ch #\() + (let ([call-start (call-name-start-before-open text i)] + [close (matching-close-paren-index text i)]) + (or (and call-start + close + (> source-start i) + (<= source-end close) + (not (and (= call-start source-start) + (= (+ close 1) source-end))) + (or focused? + (> call-start 0) + (< (+ close 1) len))) + (loop (+ i 1) state #f)))] + [else (loop (+ i 1) state #f)]))])))) + +(def (taint-safe-function-use? rule source-state container source-text) + (and (taint-assume-safe-functions? rule) + (let ([source (taint-state-finding source-state)]) + (and source + container + (finding-range-contains? container source) + (not (finding-range-equal? container source)) + (let ([container-start (finding-start-offset container)]) + (safe-function-wrapper-in-text? + (finding-text container source-text) + (- (finding-start-offset source) container-start) + (- (finding-end-offset source) container-start) + (finding-focused-to-whole-binding? container))))))) + (def (token-source-inside-function-value-sink? source-state sink source-text) (and (taint-state-token? source-state) (let ([sink-text (finding-text sink source-text)]) @@ -8833,6 +8935,7 @@ (not (and (taint-assume-safe-booleans? rule) (text-contains-comparison? (finding-text sink source-text)))) + (not (taint-safe-function-use? rule source-state sink source-text)) (not (taint-safe-index-use? rule source-state sink source-text)) (not (token-source-inside-function-value-sink? source-state @@ -8993,11 +9096,30 @@ (let ([sanitizer (taint-state-finding sanitizer-state)]) (and sanitizer from-finding - (finding-range-contains? from-finding sanitizer) - (source-state-reaches-finding? - source-state - sanitizer - source)))) + (or (and (finding-range-contains? from-finding sanitizer) + (source-state-reaches-finding? + source-state + sanitizer + source)) + (let ([source-finding + (taint-state-finding source-state)]) + (and source-finding + (same-simple-function-scope? + source + source-finding + sanitizer) + (same-simple-function-scope? + source + sanitizer + from-finding) + (finding-between? + source-finding + sanitizer + from-finding) + (source-compatible-with-sink? + source-state + sanitizer + source))))))) sanitizers)) (def (taint-propagator-blocked-by-safe-option? @@ -9013,6 +9135,11 @@ (and (taint-assume-safe-numbers? rule) from-text (text-contains-numeric-arithmetic? from-text)) + (taint-safe-function-use? + rule + source-state + from-finding + source) (taint-safe-index-use? rule source-state --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1412,6 +1412,18 @@ (check (finding-start-line (cadr findings)) => 2) (check (finding-start-line (caddr findings)) => 3))) +(test-case "scan taint assume safe functions blocks wrapper" + (let* ([taint-config + "rules:\n - id: demo.taint.safe-functions\n mode: taint\n languages: [python]\n message: safe function taint\n severity: WARNING\n options:\n taint_assume_safe_functions: true\n pattern-sources:\n - pattern: tainted(...)\n pattern-sinks:\n - patterns:\n - pattern: sink($X)\n - focus-metavariable: $X\n"] + [findings + (scan-config-string + taint-config + "python" + "demo.py" + "sink(tainted())\nsink(not_a_propagator(tainted()))\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 1))) + (test-case "scan taint implicit assignment propagation" (let* ([taint-config "rules:\n - id: demo.taint.assignment\n mode: taint\n languages: [python]\n message: assigned taint\n severity: WARNING\n pattern-sources:\n - pattern: source()\n pattern-sinks:\n - pattern: sink($X)\n"] @@ -1435,6 +1447,17 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 4))) +(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"] + [findings + (scan-config-string + taint-config + "python" + "demo.py" + "def test_function():\n id = request\n try:\n id = int(id)\n except Exception:\n raise BadRequest()\n\n some_object = id\n csv_file = get_csv(some_object)\n return send_file(csv_file)\n")]) + (check (length findings) => 0))) + (test-case "scan taint symbolic with focused sink" (let* ([taint-config "rules:\n - id: demo.taint.symbolic.with\n mode: taint\n languages: [python]\n message: symbolic focused sink\n severity: WARNING\n options:\n symbolic_propagation: true\n pattern-sources:\n - pattern: source()\n pattern-sinks:\n - patterns:\n - pattern: scoped_session(...)(...).execute($SQL, ...)\n - focus-metavariable: $SQL\n"]