Improve rust tainting by dimitris-m · Pull Request #485 · opengrep/opengrep

dimitris-m · 2025-12-09T23:48:08Z

We now correctly propagate taint when using `if let`

if let P = E {...} else {...}

For example this target:

fn f(some_x: Option<String>) {
    // ok
    let z = sink(x) 
    if let Some(x) = some_x {
        // ruleid: taint
        sink(x)
    } else {
        // ok
        sink(x)
        // should not hit since x is not defined here!
    }
}

will test as expected with this rule:

rules:
- id: taint
  mode: taint
  languages: [rust]
  message: "tainted"
  severity: INFO
  pattern-sources:
    - patterns:
        - pattern-inside: |
            fn $F(..., $P: $_, ...){...}
        - focus-metavariable: $P
  pattern-sinks:
    - pattern: sink($R)

Other fixes

The tests in tainting_tests/ were not running.
The macos-13 image is now deprecated in GHA and CI failed on main, this is now fixed by bumping to macos-14.

corneliuhoffman

Happy with it, maybe just add a let with another pattern to the test and call this G.faske "pattern" or something

corneliuhoffman · 2025-12-10T09:33:45Z

src/engine/tests/Test_engine.ml

    | Some (rules, target, xlang) -> (
        (* expected *)
-        (* not tororuleid! not ok:! not todook:
+        (* not todoruleid! not ok:! not todook:


corneliuhoffman · 2025-12-10T09:48:23Z

src/analyzing/AST_to_IL.ml

+    (* Convert to switch(e) { pat -> if_branch, _ -> else_branch }. *)
+    let cond_opt = Some (G.Cond e) in
+    let if_case_and_body =
+      G.CasesAndBody ([ G.Case (G.fake "some", pat) ], st1)


seems to me this is specially for a Some pattern no? what if it is a different pattern?

In fact I just realised that these are fine bc of Naming ... I only object to the "some" here :)

I just call it "case" now, it does not matter what name it has, and there is a function in Generic AST that uses that name.

For tainting to work, we treat Rust's LetCond, for example in code like `if let Some(x) = e {if_branch} else {else_branch}`, similarly to a LetPattern. Specifically, the above `If` becomes a `G.Switch` with a condition on `e`, then a pattern `Some(x) => if_branch` and `G.PatWilcard => else_branch`.

The tainting_rules test were not running.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [opengrep/opengrep](https://github.com/opengrep/opengrep) | minor | `v1.12.1` -> `v1.13.2` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>opengrep/opengrep (opengrep/opengrep)</summary> ### [`v1.13.2`](https://github.com/opengrep/opengrep/releases/tag/v1.13.2): Opengrep 1.13.2 [Compare Source](opengrep/opengrep@v1.13.1...v1.13.2) #### Improvements - C#: Add matching on function argument modifiers (ref, in, scoped, etc.) by [@maciejpirog](https://github.com/maciejpirog) in [#494](opengrep/opengrep#494) - C#: Support extension blocks by [@maciejpirog](https://github.com/maciejpirog) in [#496](opengrep/opengrep#496) #### Release process - Validate tag input on release by [@lae](https://github.com/lae) in [#493](opengrep/opengrep#493) **Full Changelog**: <opengrep/opengrep@v1.13.1...v1.13.2> ### [`v1.13.1`](https://github.com/opengrep/opengrep/releases/tag/v1.13.1): Opengrep 1.13.1 [Compare Source](opengrep/opengrep@v1.12.1...v1.13.1) #### Improvements - Improve handling of patterns and `AST_to_IL` translation by [@dimitris-m](https://github.com/dimitris-m) in [#483](opengrep/opengrep#483) - Improve rust tainting by [@dimitris-m](https://github.com/dimitris-m) in [#485](opengrep/opengrep#485) - Dump generic AST to HTML by [@maciejpirog](https://github.com/maciejpirog) in [#484](opengrep/opengrep#484) - Modernise C# by [@maciejpirog](https://github.com/maciejpirog) in [#487](opengrep/opengrep#487) #### Bug fixes - Fix for kotlin double-annotation bug by [@maciejpirog](https://github.com/maciejpirog) in [#480](opengrep/opengrep#480) - Fix PCRE2 test making OSX build fail by [@dimitris-m](https://github.com/dimitris-m) in [#486](opengrep/opengrep#486) - Fix: in `LetPattern(pat, e)`, `e` should be visited first by [@dimitris-m](https://github.com/dimitris-m) in [#488](opengrep/opengrep#488) #### CI fixes - Force python 3.13 for osx binary workflow by [@dimitris-m](https://github.com/dimitris-m) in [#490](opengrep/opengrep#490) ##### Notes - Version 1.13.0 ([#489](opengrep/opengrep#489)) intentionally skipped due to CI errors, fixed in [#490](opengrep/opengrep#490). **Full Changelog**: <opengrep/opengrep@v1.12.1...v1.13.1> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

dimitris-m requested review from corneliuhoffman, maciejpirog and willem-delbare as code owners December 9, 2025 23:48

dimitris-m removed the request for review from willem-delbare December 9, 2025 23:48

dimitris-m added taint lang Add or improve language support labels Dec 9, 2025

corneliuhoffman approved these changes Dec 10, 2025

View reviewed changes

dimitris-m added 4 commits December 10, 2025 12:18

fix tainting tests; add rust if let test

1ff2955

The tainting_rules test were not running.

fix: macos-13 images deprecated, bump to macos-14

4bf222b

fix reference semgrep => opengrep in error msg

184a394

dimitris-m force-pushed the dm/fix-rust-letcond-ast-to-il branch from ae7369b to 184a394 Compare December 10, 2025 12:18

dimitris-m merged commit ce1d343 into main Dec 10, 2025
3 checks passed

dimitris-m deleted the dm/fix-rust-letcond-ast-to-il branch December 10, 2025 12:28

dimitris-m mentioned this pull request Dec 15, 2025

Release v1.13.0 #489

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve rust tainting#485

Improve rust tainting#485
dimitris-m merged 4 commits intomainfrom
dm/fix-rust-letcond-ast-to-il

dimitris-m commented Dec 9, 2025 •

edited

Loading

Uh oh!

corneliuhoffman left a comment

Uh oh!

corneliuhoffman Dec 10, 2025

Uh oh!

corneliuhoffman Dec 10, 2025

Uh oh!

corneliuhoffman Dec 10, 2025

Uh oh!

dimitris-m Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dimitris-m commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

We now correctly propagate taint when using if let

Other fixes

Uh oh!

corneliuhoffman left a comment

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

dimitris-m Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dimitris-m commented Dec 9, 2025 •

edited

Loading

We now correctly propagate taint when using `if let`