Reach taint through multiline call sinks and bare-metavar sources
ober
32bf342db2db7ac4d3ac42be507da845e641226a
--- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -900,6 +900,15 @@ (generic-plain-ellipsis-at? source (generic-skip-whitespace source (+ i 1))))) + (def (generic-ellipsis-inside-parens? source i) + (let loop ([j 0] [depth 0]) + (cond + [(>= j i) (> depth 0)] + [(char=? (string-ref source j) #\() + (loop (+ j 1) (+ depth 1))] + [(char=? (string-ref source j) #\)) + (loop (+ j 1) (max 0 (- depth 1)))] + [else (loop (+ j 1) depth)]))) (def (generic-word-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -1016,7 +1025,11 @@ (char=? (string-ref source (+ i 2)) #\.)) (loop (generic-skip-whitespace source (+ i 3)) - (cons plain-ellipsis parts) + (cons + (if (generic-ellipsis-inside-parens? source i) + "(?:.|\\n)*?" + plain-ellipsis) + parts) captures)] [(generic-word-char? (string-ref source i)) (let word-loop ([j (+ i 1)]) @@ -31351,6 +31364,17 @@ (<= (metavariable-binding-end-byte binding) other-end) (string=? text (metavariable-binding-text binding))))) (finding-metavars other)))) + (def (finding-metavars-all-self? finding) + (let ([fs (finding-start-offset finding)] + [fe (finding-end-offset finding)] + [mvars (finding-metavars finding)]) + (and (not (null? mvars)) + (let loop ([entries mvars]) + (or (null? entries) + (let ([binding (cdr (car entries))]) + (and (= (metavariable-binding-start-byte binding) fs) + (= (metavariable-binding-end-byte binding) fe) + (loop (cdr entries))))))))) (def (identifier-token-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -32446,7 +32470,10 @@ (and (taint-state-token? source-state) (finding-range-contains? sink source) (not (finding-range-equal? sink source)) - (null? (finding-metavars source))) + (or (null? (finding-metavars source)) + (and (not (taint-state-side-effect? + source-state)) + (finding-metavars-all-self? source)))) (and (not (taint-state-token? source-state)) (direct-call-argument-source? source --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,11 +3,11 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "6982E07679D20836") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "3214FDA05ECD812B") - ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/scan.ss" . "71FE8A8073E6A24A") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") - ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/rule.ss" . "E12C108153C181FA") ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") + ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") ("src/semgrep/match/structural.ss" . "6FE77014EE9FDCE4") ("src/semgrep/main.ss" . "A4EC9E7F2A09D25E") ("src/semgrep/cli.ss" . "EBDC4B1DAD3F13CC")) --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -999,6 +999,17 @@ source (generic-skip-whitespace source (+ i 1))))) +;; An ellipsis inside an open call/parenthesis should match across newlines, so +;; `sink(...)` matches a call whose arguments span multiple lines. Outside +;; parens the line-bounded form is kept to avoid over-matching across statements. +(def (generic-ellipsis-inside-parens? source i) + (let loop ([j 0] [depth 0]) + (cond + [(>= j i) (> depth 0)] + [(char=? (string-ref source j) #\() (loop (+ j 1) (+ depth 1))] + [(char=? (string-ref source j) #\)) (loop (+ j 1) (max 0 (- depth 1)))] + [else (loop (+ j 1) depth)]))) + (def (generic-word-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -1103,7 +1114,10 @@ (char=? (string-ref source (+ i 1)) #\.) (char=? (string-ref source (+ i 2)) #\.)) (loop (generic-skip-whitespace source (+ i 3)) - (cons plain-ellipsis parts) + (cons (if (generic-ellipsis-inside-parens? source i) + "(?:.|\\n)*?" + plain-ellipsis) + parts) captures)] [(generic-word-char? (string-ref source i)) (let word-loop ([j (+ i 1)]) @@ -31318,6 +31332,22 @@ (string=? text (metavariable-binding-text binding))))) (finding-metavars other)))) +;; True when a finding is exactly a single metavariable (every binding spans the +;; whole finding), i.e. a bare `$X` pattern. Such a source carries a trivial +;; self-binding, so it should be treated like a metavariable-free token for +;; source-in-sink reachability. +(def (finding-metavars-all-self? finding) + (let ([fs (finding-start-offset finding)] + [fe (finding-end-offset finding)] + [mvars (finding-metavars finding)]) + (and (not (null? mvars)) + (let loop ([entries mvars]) + (or (null? entries) + (let ([binding (cdr (car entries))]) + (and (= (metavariable-binding-start-byte binding) fs) + (= (metavariable-binding-end-byte binding) fe) + (loop (cdr entries))))))))) + (def (identifier-token-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -32337,7 +32367,12 @@ (and (taint-state-token? source-state) (finding-range-contains? sink source) (not (finding-range-equal? sink source)) - (null? (finding-metavars source))) + (or (null? (finding-metavars source)) + ;; A bare `$X` source (self-binding) contained in a sink + ;; reaches it, except by-side-effect sources whose own + ;; occurrence is not a hit on the containing sink. + (and (not (taint-state-side-effect? source-state)) + (finding-metavars-all-self? source)))) (and (not (taint-state-token? source-state)) (direct-call-argument-source? source sink source-text)) (and (finding-range-contains? sink source) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -4309,6 +4309,18 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 3))) +(test-case "scan PHP taint reaches multiline call sink with interpolated source" + (let* ([taint-config + "rules:\n - id: demo.taint.php.multiline\n mode: taint\n languages: [php]\n message: multiline sink\n severity: WARNING\n pattern-sources:\n - pattern: $foo\n pattern-sinks:\n - pattern: sink(...)\n"] + [findings + (scan-config-string + taint-config + "php" + "demo.php" + "<?php\nfunction f() {\n sink(\n // x\n \"$foo\" . 'aaa'\n );\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 3))) + (test-case "scan JavaScript taint sanitizer after verify stays in function scope" (let* ([taint-config "rules:\n - id: demo.taint.jwt.verify\n mode: taint\n languages: [javascript]\n message: jwt token\n severity: WARNING\n pattern-sources:\n - pattern: $TOKEN\n pattern-sanitizers:\n - patterns:\n - pattern-inside: |\n $JWT.verify($TOKEN, ...)\n ...\n - pattern: $TOKEN\n pattern-sinks:\n - patterns:\n - pattern: $JWT.decode($TOKEN, ...)\n - pattern: $TOKEN\n"]