Match generic call-argument metavariables and focus-aware sink taint
ober
3d3d0e69b43baff506afb202d4798350aea7f1bd
--- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -835,6 +835,19 @@ "(?:\\s+|//[^\\n]*(?:\\n|$)|/\\*(?:.|\\n)*?\\*/)+") (def generic-optional-separator-regex "(?:\\s+|//[^\\n]*(?:\\n|$)|/\\*(?:.|\\n)*?\\*/)*") + (def generic-metavariable-regex + "(\\$?[A-Za-z0-9_]+|'[^'\\n]*'|\"[^\"\\n]*\")") + (def generic-metavariable-identifier-regex + "(\\$?[A-Za-z0-9_]+)") + (def (generic-metavariable-in-argument-position? source i) + (let loop ([j (- i 1)]) + (cond + [(< j 0) #f] + [(char-whitespace? (string-ref source j)) (loop (- j 1))] + [(or (char=? (string-ref source j) #\() + (char=? (string-ref source j) #\,)) + #t] + [else #f]))) (def (regex-special-char? ch) (let loop ([xs '(#\\ #\. #\^ #\$ #\| #\? #\* #\+ #\( #\) #\[ #\] #\{ #\})]) @@ -881,6 +894,12 @@ (if (and (< j len) (char-whitespace? (string-ref source j))) (loop (+ j 1)) j)))) + (def (generic-comma-ellipsis-at? source i) + (and (< i (string-length source)) + (char=? (string-ref source i) #\,) + (generic-plain-ellipsis-at? + source + (generic-skip-whitespace source (+ i 1))))) (def (generic-word-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -969,10 +988,28 @@ (name-loop (+ j 1)) (loop j - (cons "([A-Za-z0-9_]+)" parts) + (cons + (if (generic-metavariable-in-argument-position? + source + i) + generic-metavariable-regex + generic-metavariable-identifier-regex) + parts) (cons (generic-capture-name source i (+ i 1) j) captures))))] + [(generic-comma-ellipsis-at? source i) + (let ([after (generic-skip-whitespace + source + (+ (generic-skip-whitespace source (+ i 1)) + 3))]) + (loop + after + (cons + (string-append "(?:" generic-optional-separator-regex "," + generic-optional-separator-regex plain-ellipsis ")?") + parts) + captures))] [(and (< (+ i 2) len) (char=? (string-ref source i) #\.) (char=? (string-ref source (+ i 1)) #\.) @@ -30027,16 +30064,20 @@ (and (alist-ref/default state 'contained #f) #t)) (def (taint-state-control? state) (and (alist-ref/default state 'control #f) #t)) + (def (taint-state-side-effect? state) + (and (alist-ref/default state 'side-effect #f) #t)) (def (taint-state-origin state) (alist-ref/default state 'origin (taint-state-finding state))) (def (taint-state-with-labels state labels) - (taint-state* (taint-state-finding state) labels - (taint-state-exact? state) (taint-state-token? state) - (taint-state-contained? state) (taint-state-control? state) - (taint-state-origin state))) + (cons + (cons 'side-effect (taint-state-side-effect? state)) + (taint-state* (taint-state-finding state) labels + (taint-state-exact? state) (taint-state-token? state) + (taint-state-contained? state) (taint-state-control? state) + (taint-state-origin state)))) (def (taint-by-side-effect? value) (cond [(not value) #f] @@ -30087,12 +30128,14 @@ (def (taint-state-for-spec spec finding default-label) (let ([side-effect? (taint-by-side-effect? (alist-ref/default spec 'by-side-effect #f))]) - (taint-state finding (taint-labels-for-spec spec default-label) - (alist-ref/default spec 'exact #f) - (or side-effect? - (taint-function-parameter-source-spec? spec) - (taint-bare-identifier-source-spec? spec)) - #f (alist-ref/default spec 'control #f)))) + (cons + (cons 'side-effect side-effect?) + (taint-state finding (taint-labels-for-spec spec default-label) + (alist-ref/default spec 'exact #f) + (or side-effect? + (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)]) @@ -31297,10 +31340,16 @@ (let ([text (source-slice source (finding-start-offset finding) - (finding-end-offset finding))]) + (finding-end-offset finding))] + [other-start (finding-start-offset other)] + [other-end (finding-end-offset other)]) (any? (lambda (entry) - (string=? text (metavariable-binding-text (cdr entry)))) + (let ([binding (cdr entry)]) + (and (>= (metavariable-binding-start-byte binding) + other-start) + (<= (metavariable-binding-end-byte binding) other-end) + (string=? text (metavariable-binding-text binding))))) (finding-metavars other)))) (def (identifier-token-char? ch) (or (char-alphabetic? ch) @@ -32372,6 +32421,8 @@ sink) (finding-range-contains? sink source))) (source-state-before-sink? source-state source sink) + (and (not (taint-state-side-effect? source-state)) + (finding-range-equal? source sink)) (and (taint-state-contained? source-state) (finding-range-contains? sink source)) (and (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" . "6982E07679D20836") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "9BC9ED2F515D91C6") - ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") + ("src/semgrep/scan.ss" . "3214FDA05ECD812B") ("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" . "6FE77014EE9FDCE4") ("src/semgrep/main.ss" . "A4EC9E7F2A09D25E") ("src/semgrep/cli.ss" . "EBDC4B1DAD3F13CC")) --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -912,6 +912,27 @@ (def generic-optional-separator-regex "(?:\\s+|//[^\\n]*(?:\\n|$)|/\\*(?:.|\\n)*?\\*/)*") +;; A metavariable normally matches a bare or `$`-prefixed identifier. In +;; argument position (right after `(` or `,`) it may also be a single quoted +;; string, so call patterns like `f($X, ...)` can bind a string literal. The +;; string form is NOT allowed elsewhere, or a bare `$EXPR` pattern would match +;; string literals it should not. +(def generic-metavariable-regex + "(\\$?[A-Za-z0-9_]+|'[^'\\n]*'|\"[^\"\\n]*\")") + +(def generic-metavariable-identifier-regex + "(\\$?[A-Za-z0-9_]+)") + +(def (generic-metavariable-in-argument-position? source i) + (let loop ([j (- i 1)]) + (cond + [(< j 0) #f] + [(char-whitespace? (string-ref source j)) (loop (- j 1))] + [(or (char=? (string-ref source j) #\() + (char=? (string-ref source j) #\,)) + #t] + [else #f]))) + (def (regex-special-char? ch) (let loop ([xs '(#\\ #\. #\^ #\$ #\| #\? #\* #\+ #\( #\) #\[ #\] #\{ #\})]) (and (not (null? xs)) @@ -968,6 +989,16 @@ (loop (+ j 1)) j)))) +;; `, ...` in an argument list means "and zero or more further arguments", so +;; the comma and trailing ellipsis are together optional: `f($X, ...)` matches +;; both `f(a)` and `f(a, b)`. +(def (generic-comma-ellipsis-at? source i) + (and (< i (string-length source)) + (char=? (string-ref source i) #\,) + (generic-plain-ellipsis-at? + source + (generic-skip-whitespace source (+ i 1))))) + (def (generic-word-char? ch) (or (char-alphabetic? ch) (char-numeric? ch) @@ -1048,9 +1079,25 @@ (char=? (string-ref source j) #\_))) (name-loop (+ j 1)) (loop j - (cons "([A-Za-z0-9_]+)" parts) + (cons (if (generic-metavariable-in-argument-position? source i) + generic-metavariable-regex + generic-metavariable-identifier-regex) + parts) (cons (generic-capture-name source i (+ i 1) j) captures))))] + [(generic-comma-ellipsis-at? source i) + (let ([after (generic-skip-whitespace + source + (+ (generic-skip-whitespace source (+ i 1)) 3))]) + (loop after + (cons (string-append "(?:" + generic-optional-separator-regex + "," + generic-optional-separator-regex + plain-ellipsis + ")?") + parts) + captures))] [(and (< (+ i 2) len) (char=? (string-ref source i) #\.) (char=? (string-ref source (+ i 1)) #\.) @@ -30045,18 +30092,22 @@ (def (taint-state-control? state) (and (alist-ref/default state 'control #f) #t)) +(def (taint-state-side-effect? state) + (and (alist-ref/default state 'side-effect #f) #t)) + (def (taint-state-origin state) (alist-ref/default state 'origin (taint-state-finding state))) (def (taint-state-with-labels state labels) - (taint-state* - (taint-state-finding state) - labels - (taint-state-exact? state) - (taint-state-token? state) - (taint-state-contained? state) - (taint-state-control? state) - (taint-state-origin state))) + (cons (cons 'side-effect (taint-state-side-effect? state)) + (taint-state* + (taint-state-finding state) + labels + (taint-state-exact? state) + (taint-state-token? state) + (taint-state-contained? state) + (taint-state-control? state) + (taint-state-origin state)))) (def (taint-by-side-effect? value) (cond @@ -30116,15 +30167,17 @@ (let ([side-effect? (taint-by-side-effect? (alist-ref/default spec 'by-side-effect #f))]) - (taint-state - finding - (taint-labels-for-spec spec default-label) - (alist-ref/default spec 'exact #f) - (or side-effect? - (taint-function-parameter-source-spec? spec) - (taint-bare-identifier-source-spec? spec)) - #f - (alist-ref/default spec 'control #f)))) + (cons + (cons 'side-effect side-effect?) + (taint-state + finding + (taint-labels-for-spec spec default-label) + (alist-ref/default spec 'exact #f) + (or side-effect? + (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)] @@ -31252,9 +31305,17 @@ (def (finding-text-equals-any-binding? finding other source) (let ([text (source-slice source (finding-start-offset finding) - (finding-end-offset finding))]) + (finding-end-offset finding))] + [other-start (finding-start-offset other)] + [other-end (finding-end-offset other)]) (any? (lambda (entry) - (string=? text (metavariable-binding-text (cdr entry)))) + ;; Only bindings within `other`'s own range count: when a sink is + ;; focused to one metavariable, a sibling metavariable bound + ;; elsewhere in the match (e.g. an argument) is not part of the sink. + (let ([binding (cdr entry)]) + (and (>= (metavariable-binding-start-byte binding) other-start) + (<= (metavariable-binding-end-byte binding) other-end) + (string=? text (metavariable-binding-text binding))))) (finding-metavars other)))) (def (identifier-token-char? ch) @@ -32247,6 +32308,12 @@ (source-state-before-sink? source-state source sink) (finding-range-contains? sink source))) (source-state-before-sink? source-state source sink) + ;; A value that is itself a source AND the focused sink (same + ;; range) reaches the sink, e.g. `$TOKEN` as both source and sink. + ;; By-side-effect sources are excluded: they taint a later use, so + ;; the source occurrence itself is not a hit on the same sink. + (and (not (taint-state-side-effect? source-state)) + (finding-range-equal? source sink)) (and (taint-state-contained? source-state) (finding-range-contains? sink source)) (and (finding-range-contains? sink source) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -4297,6 +4297,18 @@ (check (length findings) => 1) (check (finding-start-line (car findings)) => 3))) +(test-case "scan PHP taint hits focused receiver sink, not untainted receiver" + (let* ([taint-config + "rules:\n - id: demo.taint.php.lval\n mode: taint\n languages: [php]\n message: lval sink\n severity: INFO\n pattern-sources:\n - pattern: new DOMDocument(...)\n pattern-sinks:\n - patterns:\n - pattern-inside: $DOMDOCUMENT->load($FILENAME, ...)\n - pattern: $DOMDOCUMENT\n"] + [findings + (scan-config-string + taint-config + "php" + "demo.php" + "<?php\n$doc = new DOMDocument();\n$doc->load('file.xml');\n$other->load($doc);\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"]