feat(taint): sink-side focus-metavariable + pattern-inside (→ 95.7%)#534
Conversation
…e + call-context pattern-inside) The sink-side analog of the parameter-as-source shape (PR #533). A common rejected sink shape names a focused metavariable that is an ARGUMENT of a call given by a pattern-inside/pattern context, e.g. pattern-sinks: - patterns: - pattern: assert($SINK, ...) - pattern: $SINK # focus pattern-sinks: - patterns: - focus-metavariable: $QUERY - pattern-either: - pattern-inside: $POOL.query($QUERY, ...) - pattern-inside: $POOL.execute($QUERY, ...) Semgrep means "the focused argument, when it appears inside this call, is a sink." The dropped-constraint fallback empties the sink role (no bare metavar is a usable sink), rejecting the rule. Recognise this shape and compile the call context to the existing Call / MethodName sink matchers for the call's callee: - a concrete callee (assert, redirect_to, YAML.load) -> one Call; - a $RECV.$METH(...) callee pinned by an anchored-alternation metavariable-regex (^(query|execute)$, \b(include|require)\b) -> one MethodName per listed name; - a $FUNC(...) pure-metavariable callee pinned by such a regex -> one Call per name. These reuse the existing taint-gated call sinks, which fire only when a tracked value reaches the call's arguments, so the compiled sink is bounded to the concrete callee/method name AND tainted data -- never an over-broad bare-node sink. Recognition is bounded: the focused metavariable must be in the call's argument list (or the arg list is a wildcard), and a free metavariable callee with no pinning regex produces no matcher (we never invent a name), falling through to the normal graceful-degradation extraction. No engine changes: the new logic emits only existing Call/MethodName GenericMatchers. registry coverage: 94.6% (2028) -> 95.7% (2051), +23 rules; taint unsupported-shape 102 -> 80. Adds firing + clean safe-near-miss bridge tests for PHP (concrete Call) and JS (regex-pinned MethodName), plus an over-broad guard test that an unpinned metavariable callee is not compiled.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR implements a new "focus-on-call-argument" sink/sanitizer shape in the Semgrep taint bridge ( ChangesFocus-on-call-argument sink shape
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![foxguard-app[bot]](https://avatars.githubusercontent.com/in/3854821?s=60&v=4){kind=small}
| assert($x); | ||
| } | ||
| "#; | ||
| let tree = parse_file(src, Language::Php).expect("php fixture should parse"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| assert($x); | ||
| } | ||
| "#; | ||
| let tree = parse_file(src, Language::Php).expect("php fixture should parse"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| pool.query(q); | ||
| } | ||
| "#; | ||
| let tree = parse_file(src, Language::JavaScript).expect("js fixture should parse"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| pool.format(q); | ||
| } | ||
| "#; | ||
| let tree = parse_file(src, Language::JavaScript).expect("js fixture should parse"); |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.expect() can panic at runtime — use proper error handling with ? or match
| /// name), so the rule is rejected rather than producing an any-call sink. | ||
| #[test] | ||
| fn unpinned_metavar_callee_sink_is_not_compiled() { | ||
| let v: YamlValue = serde_yaml_ng::from_str( |
There was a problem hiding this comment.
foxguard · MEDIUM · rs/no-unwrap-in-lib (CWE-248)
.unwrap() can panic at runtime — use proper error handling with ? or match
1 participant
Taint: sink-side focus-metavariable + pattern-inside (call-context) → load rate 94.6% → 95.7%
The symmetric sink-side analog of #533:
focus-metavariable: $ARG+pattern-inside: <call-context>insidepattern-sinks/pattern-sanitizers→ the focused argument of a concrete call (gated by the inside-context) is a sink. Reuses the focus-binding/signature infra from the parameter-source work; no engine changes (compiles to existingCall/MethodNamematchers).+23 rules → 95.7% (2051/2144), taint unsupported-shape 102 → 80. Step-1 upper bound was 12 distinct flippable; the realized +23 includes co-blocked rules whose source was already expressible and only the sink was missing (
node-mysql-sqli,node-knex-sqli,ruby-pg-sqli,tainted-deserialization,tainted-filename,check-unsafe-reflection,express-ssrf,express-res-sendfile,file-inclusion, thedangerous-*-tainted-env-argsfamily, etc.).Precision (guarded)
Only concrete-callee call-context sinks compile (
Call{assert},MethodName{query,execute}). A free metavar callee with nometavariable-regexpin emits nothing — guard testunpinned_metavar_callee_sink_is_not_compiled. Per-language firing + safe near-miss bridge tests: PHPassert($tainted)fires / untainted literal doesn't; JSpool.query(tainted)fires /pool.format(...)(method outside pinned set) doesn't.Verification (re-run on branch)
95.7% re-measured · both dogfood exit 0 ·
cargo test0 failed · clippy-D warningsclean · fmt clean · 3-file additive diff (+547 in semgrep_taint.rs), no engine/Cargo/baseline changes · COMPATIBILITY.md updated.Summary by CodeRabbit
New Features
Documentation