Reach taint through PHP echo/print statement sinks
ober
2c86082e60c0ce96112c23a3e904f6d8346032bb
--- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -31669,6 +31669,23 @@ (string=? (string-trim (substring sink-text (+ open 1) close)) (string-trim source-text0))))) + (def (echo-or-print-statement-sink? sink source-text) + (let ([trimmed (string-trim + (finding-text sink source-text))]) + (or (sg-string-prefix? "echo " trimmed) + (sg-string-prefix? "print " trimmed)))) + (def (source-sanitized-within-sink? + source + sanitizers + sink + source-text) + (any? + (lambda (sanitizer-state) + (let ([sanitizer (taint-state-finding sanitizer-state)]) + (and sanitizer + (finding-range-contains? sanitizer source) + (finding-range-contains? sink sanitizer)))) + sanitizers)) (def (call-name-start-before-open text open-index) (let loop ([i (- open-index 1)]) (cond @@ -32383,7 +32400,15 @@ (direct-call-argument-source? source sink - source-text))) + source-text)) + (and (finding-range-contains? sink source) + (not (finding-range-equal? sink source)) + (echo-or-print-statement-sink? sink source-text) + (not (source-sanitized-within-sink? + source + sanitizers + sink + source-text)))) (not (and (taint-assume-safe-comparisons? rule) (text-contains-comparison? (finding-text sink source-text)))) --- 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" . "3C8AA38427B2EE0A") + ("src/semgrep/scan.ss" . "9BC9ED2F515D91C6") + ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") + ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("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 @@ -31612,6 +31612,23 @@ (substring sink-text (+ open 1) close)) (string-trim source-text0))))) +;; `echo EXPR;` / `print EXPR;` are statement sinks with no call parentheses, +;; so the access-path model does not see EXPR as an argument. Treat any source +;; contained in the echoed/printed expression as reaching the sink; sanitizers +;; on the expression still block it via the later sanitizer check. +(def (echo-or-print-statement-sink? sink source-text) + (let ([trimmed (string-trim (finding-text sink source-text))]) + (or (sg-string-prefix? "echo " trimmed) + (sg-string-prefix? "print " trimmed)))) + +(def (source-sanitized-within-sink? source sanitizers sink source-text) + (any? (lambda (sanitizer-state) + (let ([sanitizer (taint-state-finding sanitizer-state)]) + (and sanitizer + (finding-range-contains? sanitizer source) + (finding-range-contains? sink sanitizer)))) + sanitizers)) + (def (call-name-start-before-open text open-index) (let loop ([i (- open-index 1)]) (cond @@ -32255,7 +32272,15 @@ (not (finding-range-equal? sink source)) (null? (finding-metavars source))) (and (not (taint-state-token? source-state)) - (direct-call-argument-source? source sink source-text))) + (direct-call-argument-source? source sink source-text)) + (and (finding-range-contains? sink source) + (not (finding-range-equal? sink source)) + (echo-or-print-statement-sink? sink source-text) + (not (source-sanitized-within-sink? + source + sanitizers + sink + source-text)))) (not (and (taint-assume-safe-comparisons? rule) (text-contains-comparison? (finding-text sink source-text)))) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -4285,6 +4285,18 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 8))) +(test-case "scan PHP taint reaches echo statement sink, respects sanitizer" + (let* ([taint-config + "rules:\n - id: demo.taint.php.echo\n mode: taint\n languages: [php]\n message: echo taint\n severity: ERROR\n pattern-sources:\n - pattern: $_GET[...]\n pattern-sanitizers:\n - pattern: htmlspecialchars(...)\n pattern-sinks:\n - pattern: echo ...;\n"] + [findings + (scan-config-string + taint-config + "php" + "demo.php" + "<?php\nfunction foo() {\n echo $_GET[\"a\"];\n echo htmlspecialchars($_GET[\"b\"]);\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"]