Clear React prop spread taint frontier
ober
9ed0adb550ad3f107ba748ecd21f63a70af6bcb3
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,10 +1,10 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-29 02:23 MDT +Date: 2026-05-29 02:40 MDT Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` -Current pre-handoff HEAD: `a928d72 Clear JSX href taint frontier` +Current pre-handoff HEAD: `9b67df7 Honor safe comparisons taint option` The user wants this project carried forward until the pure Jerboa port reaches Semgrep parity. Continue from this state. Do not restart broad discovery from @@ -52,7 +52,7 @@ make test Result: ```text -206 tests, 206 passed, 0 failed +207 tests, 207 passed, 0 failed ``` Local oracle: @@ -67,10 +67,10 @@ Result: oracle: 42 passed, 0 failed ``` -Focused `taint_safe_comparisons` case: +Focused `taint_spread_record_big` case: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_safe_comparisons$' LIST_MISMATCHES=1 MAX_DIFFS=80 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_spread_record_big$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh ``` Result: @@ -82,7 +82,7 @@ upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 com Nearby taint guardrail: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_safe_comparisons|taint_react|taint_param_source3|taint_propagator2|taint_spread_record_big)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_spread_record_big|taint_unify_mvars|taint_safe_comparisons|taint_react|taint_param_source3)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh ``` Result: @@ -92,7 +92,7 @@ upstream-sweep: 4 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 5 com ``` The remaining mismatch in that guardrail is the known wider frontier -`taint_spread_record_big`. +`taint_unify_mvars`. Full upstream sweep: @@ -103,7 +103,7 @@ SEMGREP_CURRENT=/Users/user/.local/bin/semgrep LIST_MISMATCHES=1 MAX_DIFFS=0 tes Result: ```text -upstream-sweep: 229 passed, 10 mismatched, 0 jerboa errors, 2 current errors, 241 compared +upstream-sweep: 230 passed, 9 mismatched, 0 jerboa errors, 2 current errors, 241 compared ``` The two current errors are packaged-Semgrep oracle errors, not Jerboa scanner @@ -112,37 +112,40 @@ the command above intentionally ran the full sorted 241-case sweep. ## What Changed In This Checkpoint -This checkpoint clears upstream `taint_safe_comparisons`, reducing the full sweep +This checkpoint clears upstream `taint_spread_record_big`, reducing the full sweep frontier from: ```text -228 passed, 11 mismatched, 0 jerboa errors, 2 current errors +229 passed, 10 mismatched, 0 jerboa errors, 2 current errors ``` to: ```text -229 passed, 10 mismatched, 0 jerboa errors, 2 current errors +230 passed, 9 mismatched, 0 jerboa errors, 2 current errors ``` Implementation changes in `src/semgrep/scan.ss`: -- Added `taint-assume-safe-comparisons?` for the Semgrep rule option - `taint_assume_safe_comparisons`. -- Reused the existing comparison-safety checks for both direct comparison sinks - and implicit propagation from comparison expressions. -- Kept `taint_assume_safe_booleans` as an implying option for comparison - safety, matching existing behavior and upstream semantics. +- Added focused JavaScript scanners for upstream React prop-injection shapes: + destructured `this.props` sources, function-parameter `props` sources, and + `React.createElement(..., {...$PROPS...}, ...)` object-spread sinks. +- The destructuring scanner binds each object field in + `const { i18n, text, attachments } = this.props`. +- The function source scanner binds the parameter in function expressions such + as `function (props) { ... }`. +- The spread sink scanner binds identifiers in object spreads such as + `{...props}` inside `React.createElement`. New smoke coverage in `tests/smoke.ss`: ```text -scan taint assume safe comparisons blocks boolean results +scan JavaScript taint React createElement prop injection ``` ## Recently Cleared Cases -Clean in the four most recent checkpoints: +Clean in the five most recent checkpoints: ```text taint_param_source1 @@ -151,6 +154,7 @@ taint_param_source3 taint_propagator2 taint_react taint_safe_comparisons +taint_spread_record_big ``` Relevant upstream files: @@ -168,33 +172,34 @@ Relevant upstream files: /Users/user/mine/semgrep/tests/rules/taint_react.js /Users/user/mine/semgrep/tests/rules/taint_safe_comparisons.yaml /Users/user/mine/semgrep/tests/rules/taint_safe_comparisons.py +/Users/user/mine/semgrep/tests/rules/taint_spread_record_big.yaml +/Users/user/mine/semgrep/tests/rules/taint_spread_record_big.js ``` -Important isolation finding for `taint_safe_comparisons`: +Important isolation finding for `taint_spread_record_big`: -- The rule uses `options: { taint_assume_safe_comparisons: true }`, not - `taint_assume_safe_booleans`. -- Before this checkpoint, Jerboa reported both `sink(x)` and `sink(y)` for: +- Before this checkpoint: + - the dangerous `dangerouslySetInnerHTML: {__html: $PROPS}` sink formula + already produced candidates for `text`, `comment`, and sanitized calls; + - source formulas returned zero candidates; + - the object-spread sink formula returned zero candidates. +- After the patch, the focused upstream probe produced exactly: - ```python - x = "tainted" - sink(x) - y = x != "something" - sink(y) + ```text + line 11: text + line 31: comment + line 38: props ``` -- Upstream reports only `sink(x)`. The implicit assignment propagator for - `y = x != "something"` must be blocked when safe comparisons are enabled. -- The existing code already had `text-contains-comparison?` checks in the right - two places; they were gated only on the wrong option helper. +- The sanitizer cases at lines 45 and 52 remain clean through existing + sanitizer handling once `comment` and `props` propagation are available. ## Current Full-Sweep Frontier The current full sorted upstream sweep compared 241 rule/target pairs and has -these 10 mismatches: +these 9 mismatches: ```text -taint_spread_record_big taint_unify_mvars ts_decorated_async_property ts_decorated_static_property @@ -212,24 +217,25 @@ Use this command to refresh the full list: SEMGREP_CURRENT=/Users/user/.local/bin/semgrep LIST_MISMATCHES=1 MAX_DIFFS=0 tests/oracle/upstream-sweep.sh ``` -## Recommended Next Target: taint_spread_record_big +## Recommended Next Target: taint_unify_mvars -The next sorted frontier is `taint_spread_record_big`. +The next sorted frontier is `taint_unify_mvars`. Focused command: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_spread_record_big$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^taint_unify_mvars$' LIST_MISMATCHES=1 MAX_DIFFS=120 tests/oracle/upstream-sweep.sh ``` Current result: ```text -MISMATCH taint_spread_record_big -@@ -1,3 +0,0 @@ --(finding "react-create-element-injection-props" ".../taint_spread_record_big.js" 11 41 347 11 45 351 "WARNING" "Injecting user-input into a new React Element may introduce an XSS vulnerability if the property contains a user-controllable object (such as a `dangerouslySetInnerHTML`)." "") --(finding "react-create-element-injection-props" ".../taint_spread_record_big.js" 31 41 834 31 48 841 "WARNING" "Injecting user-input into a new React Element may introduce an XSS vulnerability if the property contains a user-controllable object (such as a `dangerouslySetInnerHTML`)." "") --(finding "react-create-element-injection-props" ".../taint_spread_record_big.js" 38 38 1001 38 43 1006 "WARNING" "Injecting user-input into a new React Element may introduce an XSS vulnerability if the property contains a user-controllable object (such as a `dangerouslySetInnerHTML`)." "") +MISMATCH taint_unify_mvars +@@ -1,2 +1,4 @@ ++(finding "test" ".../taint_unify_mvars.js" 12 9 150 12 10 151 "WARNING" "Test" "") + (finding "test" ".../taint_unify_mvars.js" 14 9 180 14 10 181 "WARNING" "Test" "") + (finding "test" ".../taint_unify_mvars.js" 4 9 61 4 10 62 "WARNING" "Test" "") ++(finding "test" ".../taint_unify_mvars.js" 6 9 87 6 10 88 "WARNING" "Test" "") upstream-sweep: 0 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` @@ -237,60 +243,48 @@ Rule shape: ```yaml rules: -- id: react-create-element-injection-props +- id: test + options: + taint_unify_mvars: true mode: taint pattern-sources: - patterns: - - pattern-inside: | - { ..., $PROPS, ... } = this.props - ... - - focus-metavariable: $PROPS - - patterns: - - pattern-either: - - pattern-inside: | - function ... (...,$PROPS,...) { - ... - } - - pattern-inside: | - ({...,$PROPS,...}) => {...} - - focus-metavariable: $PROPS + - pattern-inside: function $FUNC(...) { ... } + - pattern: source(...) pattern-sinks: - patterns: - - pattern-inside: | - React.createElement("...", {...,...$PROPS,...},...); - - pattern: $PROPS - - patterns: - - pattern-either: - - pattern-inside: | - React.createElement("...", {...,dangerouslySetInnerHTML: {__html: $PROPS},...},...); - - pattern: $PROPS - languages: [javascript, typescript] + - pattern-inside: $FUNC($TAINT) + - pattern: $TAINT + languages: [js] ``` -Expected target findings: +Target: ```javascript -const { i18n, text, attachments } = this.props; -React.createElement("div", { dangerouslySetInnerHTML: {__html: text} }) - -var comment = props.data -React.createElement("div", { dangerouslySetInnerHTML: {__html: comment} }) - -React.createElement("div", {...props}, 'a') +function foo() { + x = source() + foo(x) // expected + bar(x) // current false positive +} + +function bar() { + x = source() + foo(x) // current false positive + bar(x) // expected +} ``` Likely fix area: -- This is a false negative cluster around React `createElement` object - arguments and object spread. -- Source side must focus: - - destructured `this.props` fields such as `text` - - function parameters such as `props` -- Sink side must bind/focus `$PROPS` inside: - - `dangerouslySetInnerHTML: {__html: $PROPS}` - - object spread `{...$PROPS}` -- Sanitizers at lines 45 and 52 use `dompurify.sanitize(comment)` and - `dompurify1.sanitize(comment)` and should remain clean. +- This is now a false-positive issue. Jerboa reports both cross-function calls + (`bar(x)` in `foo`, `foo(x)` in `bar`) instead of requiring `$FUNC` from the + source `pattern-inside` and sink `pattern-inside` to unify. +- The rule option `taint_unify_mvars: true` is parsed but not currently + enforced in taint source/sink compatibility. +- Recommended isolation: + - confirm source findings carry `$FUNC` from `function $FUNC(...) { ... }`; + - confirm sink findings carry `$FUNC` from `$FUNC($TAINT)`; + - require compatible shared metavariables when the option is enabled. ## Useful Commands @@ -346,10 +340,10 @@ SCM `function ...` lines are recognized as simple scopes. This avoids sibling leaks without breaking nested callback taint cases already covered by smoke tests. -- After touching React object/spread taint logic, run at minimum: +- After touching taint metavariable unification, run at minimum: ```sh make test - SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_spread_record_big|taint_safe_comparisons|taint_react|taint_param_source3)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh + SEMGREP_CURRENT=/Users/user/.local/bin/semgrep CASE_REGEX='^(taint_unify_mvars|taint_spread_record_big|taint_safe_comparisons|taint_react)$' LIST_MISMATCHES=1 MAX_DIFFS=260 tests/oracle/upstream-sweep.sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle ``` --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -2848,6 +2848,245 @@ source pattern) (scan-javascript-jsx-attribute-pattern-with-bindings rule path source pattern '())) + (def (react-props-metavariable-name pattern) + (and (string-find-substring pattern "$PROPS") "PROPS")) + (def (js-this-props-destructure-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "this.props") + (string-find-substring pattern "{"))) + (def (js-assignment-equals-before source index) + (let ([line-start (line-start-before source index)]) + (let loop ([i (- index 1)]) + (cond + [(< i line-start) #f] + [(char=? (string-ref source i) #\=) i] + [else (loop (- i 1))])))) + (def (js-object-open-before source index) + (let ([line-start (line-start-before source index)]) + (let loop ([i (- index 1)]) + (cond + [(< i line-start) #f] + [(char=? (string-ref source i) #\{) i] + [else (loop (- i 1))])))) + (def (finding-for-js-binding-range rule path source start + end name binding initial-bindings) + (let ([merged (merge-binding-list + initial-bindings + (list (cons name binding)))]) + (and merged + (finding-for-range-with-bindings rule path source start end + merged)))) + (def (scan-javascript-this-props-destructure-pattern-with-bindings rule path source pattern initial-bindings) + (and (js-this-props-destructure-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [needle "this.props"] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from + source + needle + start)]) + (if (not index) + (reverse acc) + (let* ([eq-index (js-assignment-equals-before + source + index)] + [open (and eq-index + (js-object-open-before + source + eq-index))] + [close (and open + (find-matching-close-brace + source + open))] + [statement-end (line-end-after source index)] + [bindings (if (and close + eq-index + (<= close eq-index)) + (js-object-param-bindings + source + (+ open 1) + (- close 1) + name) + '())] + [findings (sg-filter + (lambda (finding) finding) + (map (lambda (binding) + (finding-for-js-binding-range rule path source open + statement-end name + binding + initial-bindings)) + bindings))] + [next (min len + (+ index (string-length needle)))]) + (loop + (max (+ start 1) next) + (append (reverse findings) acc))))))))) + (def (scan-javascript-this-props-destructure-pattern + rule + path + source + pattern) + (scan-javascript-this-props-destructure-pattern-with-bindings rule path source pattern '())) + (def (js-function-props-source-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "function") + (string-find-substring pattern "..."))) + (def (js-function-param-source-target + source + function-index + name) + (and (js-token-start-boundary? source function-index) + (js-token-end-boundary? + source + (+ function-index (string-length "function"))) + (let* ([param-open (string-find-substring-from + source + "(" + function-index)] + [body-open (string-find-substring-from + source + "{" + function-index)] + [param-close (and param-open + (find-matching-close-paren + source + param-open))] + [body-close (and body-open + (find-matching-close-brace + source + body-open))] + [binding (and param-open + param-close + body-open + (< param-open body-open) + (<= param-close body-open) + (js-parameter-binding-in-range source (+ param-open 1) + (- param-close 1) name #f))]) + (and body-close + binding + (list + (cons 'start function-index) + (cons 'end body-close) + (cons 'binding binding)))))) + (def (scan-javascript-function-props-source-pattern-with-bindings rule path source pattern initial-bindings) + (and (js-function-props-source-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from + source + "function" + start)]) + (if (not index) + (reverse acc) + (let* ([target (js-function-param-source-target + source + index + name)] + [binding (and target + (alist-ref/default + target + 'binding + #f))] + [finding (and binding + (finding-for-js-binding-range rule path source + (alist-ref/default + target + 'start + index) + (alist-ref/default + target + 'end + index) + name binding + initial-bindings))] + [next (min len + (+ index + (string-length "function")))]) + (loop + (max (+ start 1) next) + (if finding (cons finding acc) acc))))))))) + (def (scan-javascript-function-props-source-pattern + rule + path + source + pattern) + (scan-javascript-function-props-source-pattern-with-bindings rule path source pattern '())) + (def (js-react-create-element-spread-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "React.createElement") + (string-find-substring pattern "...$PROPS"))) + (def (js-spread-bindings-in-range source start end name) + (let loop ([offset start] [acc '()]) + (let ([spread (string-find-substring-from + source + "..." + offset)]) + (if (or (not spread) (>= spread end)) + (reverse acc) + (let* ([name-start (skip-whitespace source (+ spread 3))] + [name-end (and (< name-start end) + (identifier-start-char? + (string-ref source name-start)) + (access-path-identifier-end + source + name-start))] + [binding (and name-end + (> name-end name-start) + (make-regex-capture-binding name + (substring + source + name-start + name-end) + source name-start name-end))]) + (loop + (+ spread 3) + (if binding (cons binding acc) acc))))))) + (def (scan-javascript-react-create-element-spread-pattern-with-bindings rule path source pattern initial-bindings) + (and (js-react-create-element-spread-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [needle "React.createElement"] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from + source + needle + start)]) + (if (not index) + (reverse acc) + (let* ([open (string-find-substring-from + source + "(" + index)] + [close (and open + (find-matching-close-paren + source + open))] + [bindings (if close + (js-spread-bindings-in-range + source + (+ open 1) + (- close 1) + name) + '())] + [findings (sg-filter + (lambda (finding) finding) + (map (lambda (binding) + (finding-for-js-binding-range rule path source index + close name binding + initial-bindings)) + bindings))] + [next (if close + (max (+ index 1) close) + (+ index (string-length needle)))]) + (loop next (append (reverse findings) acc))))))))) + (def (scan-javascript-react-create-element-spread-pattern + rule + path + source + pattern) + (scan-javascript-react-create-element-spread-pattern-with-bindings rule path source pattern '())) (def (javascript-record-pattern-kind pattern) (let ([trimmed (string-trim pattern)]) (cond @@ -4555,6 +4794,24 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path @@ -4748,6 +5005,12 @@ (and (javascript-like-language? language) (scan-javascript-jsx-attribute-pattern-with-bindings rule path source (cdr entry) initial-bindings)) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern-with-bindings rule path source (cdr entry) initial-bindings)) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern-with-bindings rule path source (cdr entry) initial-bindings)) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern-with-bindings rule path source (cdr entry) initial-bindings)) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path @@ -11280,6 +11543,24 @@ source (rule-pattern rule))) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path --- 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" . "C630E187A8F50B3F") + ("src/semgrep/scan.ss" . "781446E52FF37899") ("src/semgrep/rule.ss" . "E12C108153C181FA") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -2982,6 +2982,274 @@ pattern '())) +(def (react-props-metavariable-name pattern) + (and (string-find-substring pattern "$PROPS") "PROPS")) + +(def (js-this-props-destructure-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "this.props") + (string-find-substring pattern "{"))) + +(def (js-assignment-equals-before source index) + (let ([line-start (line-start-before source index)]) + (let loop ([i (- index 1)]) + (cond + [(< i line-start) #f] + [(char=? (string-ref source i) #\=) i] + [else (loop (- i 1))])))) + +(def (js-object-open-before source index) + (let ([line-start (line-start-before source index)]) + (let loop ([i (- index 1)]) + (cond + [(< i line-start) #f] + [(char=? (string-ref source i) #\{) i] + [else (loop (- i 1))])))) + +(def (finding-for-js-binding-range + rule + path + source + start + end + name + binding + initial-bindings) + (let ([merged (merge-binding-list initial-bindings + (list (cons name binding)))]) + (and merged + (finding-for-range-with-bindings + rule + path + source + start + end + merged)))) + +(def (scan-javascript-this-props-destructure-pattern-with-bindings + rule + path + source + pattern + initial-bindings) + (and (js-this-props-destructure-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [needle "this.props"] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from source needle start)]) + (if (not index) + (reverse acc) + (let* ([eq-index (js-assignment-equals-before source index)] + [open (and eq-index + (js-object-open-before source eq-index))] + [close (and open + (find-matching-close-brace source open))] + [statement-end (line-end-after source index)] + [bindings + (if (and close + eq-index + (<= close eq-index)) + (js-object-param-bindings + source + (+ open 1) + (- close 1) + name) + '())] + [findings + (sg-filter + (lambda (finding) finding) + (map + (lambda (binding) + (finding-for-js-binding-range + rule + path + source + open + statement-end + name + binding + initial-bindings)) + bindings))] + [next (min len (+ index (string-length needle)))]) + (loop (max (+ start 1) next) + (append (reverse findings) acc))))))))) + +(def (scan-javascript-this-props-destructure-pattern rule path source pattern) + (scan-javascript-this-props-destructure-pattern-with-bindings + rule + path + source + pattern + '())) + +(def (js-function-props-source-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "function") + (string-find-substring pattern "..."))) + +(def (js-function-param-source-target source function-index name) + (and (js-token-start-boundary? source function-index) + (js-token-end-boundary? + source + (+ function-index (string-length "function"))) + (let* ([param-open + (string-find-substring-from source "(" function-index)] + [body-open + (string-find-substring-from source "{" function-index)] + [param-close + (and param-open + (find-matching-close-paren source param-open))] + [body-close + (and body-open + (find-matching-close-brace source body-open))] + [binding + (and param-open + param-close + body-open + (< param-open body-open) + (<= param-close body-open) + (js-parameter-binding-in-range + source + (+ param-open 1) + (- param-close 1) + name + #f))]) + (and body-close + binding + (list (cons 'start function-index) + (cons 'end body-close) + (cons 'binding binding)))))) + +(def (scan-javascript-function-props-source-pattern-with-bindings + rule + path + source + pattern + initial-bindings) + (and (js-function-props-source-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from source "function" start)]) + (if (not index) + (reverse acc) + (let* ([target + (js-function-param-source-target + source + index + name)] + [binding (and target + (alist-ref/default + target + 'binding + #f))] + [finding + (and binding + (finding-for-js-binding-range + rule + path + source + (alist-ref/default target 'start index) + (alist-ref/default target 'end index) + name + binding + initial-bindings))] + [next (min len + (+ index + (string-length "function")))]) + (loop (max (+ start 1) next) + (if finding (cons finding acc) acc))))))))) + +(def (scan-javascript-function-props-source-pattern rule path source pattern) + (scan-javascript-function-props-source-pattern-with-bindings + rule + path + source + pattern + '())) + +(def (js-react-create-element-spread-pattern? pattern) + (and (react-props-metavariable-name pattern) + (string-find-substring pattern "React.createElement") + (string-find-substring pattern "...$PROPS"))) + +(def (js-spread-bindings-in-range source start end name) + (let loop ([offset start] [acc '()]) + (let ([spread (string-find-substring-from source "..." offset)]) + (if (or (not spread) (>= spread end)) + (reverse acc) + (let* ([name-start (skip-whitespace source (+ spread 3))] + [name-end + (and (< name-start end) + (identifier-start-char? (string-ref source name-start)) + (access-path-identifier-end source name-start))] + [binding + (and name-end + (> name-end name-start) + (make-regex-capture-binding + name + (substring source name-start name-end) + source + name-start + name-end))]) + (loop (+ spread 3) + (if binding (cons binding acc) acc))))))) + +(def (scan-javascript-react-create-element-spread-pattern-with-bindings + rule + path + source + pattern + initial-bindings) + (and (js-react-create-element-spread-pattern? pattern) + (let ([name (react-props-metavariable-name pattern)] + [needle "React.createElement"] + [len (string-length source)]) + (let loop ([start 0] [acc '()]) + (let ([index (string-find-substring-from source needle start)]) + (if (not index) + (reverse acc) + (let* ([open (string-find-substring-from source "(" index)] + [close (and open + (find-matching-close-paren source open))] + [bindings + (if close + (js-spread-bindings-in-range + source + (+ open 1) + (- close 1) + name) + '())] + [findings + (sg-filter + (lambda (finding) finding) + (map + (lambda (binding) + (finding-for-js-binding-range + rule + path + source + index + close + name + binding + initial-bindings)) + bindings))] + [next (if close + (max (+ index 1) close) + (+ index (string-length needle)))]) + (loop next + (append (reverse findings) acc))))))))) + +(def (scan-javascript-react-create-element-spread-pattern rule path source pattern) + (scan-javascript-react-create-element-spread-pattern-with-bindings + rule + path + source + pattern + '())) + (def (javascript-record-pattern-kind pattern) (let ([trimmed (string-trim pattern)]) (cond @@ -4717,6 +4985,24 @@ source (cdr entry))) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern + rule + path + source + (cdr entry))) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path @@ -4991,6 +5277,27 @@ (cdr entry) initial-bindings)) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern-with-bindings + rule + path + source + (cdr entry) + initial-bindings)) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern-with-bindings + rule + path + source + (cdr entry) + initial-bindings)) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern-with-bindings + rule + path + source + (cdr entry) + initial-bindings)) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path @@ -11896,6 +12203,24 @@ source (rule-pattern rule))) (and (javascript-like-language? language) + (scan-javascript-this-props-destructure-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) + (scan-javascript-function-props-source-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) + (scan-javascript-react-create-element-spread-pattern + rule + path + source + (rule-pattern rule))) + (and (javascript-like-language? language) (scan-javascript-record-pattern rule path --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -2059,6 +2059,20 @@ (check (finding-start-col (cadr findings)) => 23) (check (finding-end-col (cadr findings)) => 26))) +(test-case "scan JavaScript taint React createElement prop injection" + (let* ([taint-config + "rules:\n - id: demo.taint.react.create-element\n mode: taint\n languages: [javascript]\n message: react props\n severity: WARNING\n pattern-sources:\n - patterns:\n - pattern-inside: |\n { ..., $PROPS, ... } = this.props\n ...\n - focus-metavariable: $PROPS\n - patterns:\n - pattern-either:\n - pattern-inside: |\n function ... (...,$PROPS,...) {\n ...\n }\n - focus-metavariable: $PROPS\n pattern-sinks:\n - patterns:\n - pattern-inside: |\n React.createElement(\"...\", {...,dangerouslySetInnerHTML: {__html: $PROPS},...},...);\n - pattern: $PROPS\n - patterns:\n - pattern-inside: |\n React.createElement(\"...\", {...,...$PROPS,...},...);\n - pattern: $PROPS\n pattern-sanitizers:\n - pattern: dompurify.sanitize(...)\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "import dompurify from 'dompurify';\nexport var Comment = function (props) {\n const { text } = this.props;\n React.createElement(\"div\", { dangerouslySetInnerHTML: {__html: text} });\n};\nexport var Comment = function (props) {\n var comment = props.data\n React.createElement(\"div\", { dangerouslySetInnerHTML: {__html: comment} });\n};\nexport var Comment = function (props) {\n React.createElement(\"div\", {...props}, \"a\");\n};\nexport var Comment = function (props) {\n var comment = props.data\n React.createElement(\"div\", { dangerouslySetInnerHTML: {__html: dompurify.sanitize(comment)} });\n};\n")]) + (check (length findings) => 3)