Clojure translation that supports tainting by dimitris-m · Pull Request #501 · opengrep/opengrep

dimitris-m · 2025-12-23T11:31:49Z

Pre-requisites

Add deep expression semgrep-clojure#1

Description

Each commit is self-contained.
We support (reasonably well) a large part of clojure syntax including common macros like ->, ->>, as-> etc.
There will be part 2 soon.

corneliuhoffman · 2025-12-29T10:09:12Z

languages/lisp/tree-sitter/Macro_expand_clojure.ml

+ * is not a valid binding form. We could simply expand once here
+ * for such cases... but it's simply not worth the effort.
+ *)
+let insert_threaded insert_pos v e =


oh man can we rename this e ? {e with e =...} messes with my head

corneliuhoffman · 2025-12-29T10:14:12Z

languages/lisp/tree-sitter/Macro_expand_clojure.ml

+    Macroexpansion_error ("Threading binding forms is not implemented.",
+                          G.E e)
+  | G.Lambda _ ->
+    (* Actually ((->> 4 (fn [x])) 1) => 4 but is there a sane person


I think this is not unheard of if you chain a lot of functions that are thread-first but at one point one of them is thread_last

Indeed his works now with the eager approach to macroexpansion for threaded forms.

corneliuhoffman · 2025-12-29T10:23:06Z

languages/lisp/tree-sitter/Macro_expand_clojure.ml

+        match List.rev tests_and_exprs with
+        | G.E last_expr :: G.E last_test :: rest_rev ->
+          (last_expr, last_test, List.rev rest_rev)
+        | _ -> assert false


some Log of why it failed?

Ah good point, here we could in fact have 1 expression or test (user error) I think, need to look into this.

But: this is not used now, in favor of direct macroexpansion (see map_cond_thread_first_last_form in Parse_clojure_tree_sitter.ml).

corneliuhoffman · 2025-12-29T10:48:22Z

languages/lisp/tree-sitter/Macro_expand_clojure.ml

+      G.OtherExpr (("ExprBlock", G.fake "cond-threaded-block"), body_exprs)
+      |> G.e
+    end
+


this is a bit hard to read and has several similar assert false. If I underand this correctly you do the following:
separate the (last_test, last_expr) pair, then split the begining into pairs (test, cond), then traverse that list creating "lets" and "ifs" as needed and at the end you make a final if.

Is it not more natural to split the entire list into a list (test, expr) like you do (with a log if it is odd length) and then recursively run on it and if it is long you create the let and if from the head and add to the result of recursion and if it is only one element you close? I think it will be shorter and easier to read perhaps? This is of course my nitpick, not necessary now

Yes, it is...

It's not used now, there's an equally unreadable version in the main Parse_ file!

corneliuhoffman · 2025-12-29T11:00:26Z

languages/lisp/tree-sitter/Macro_expand_clojure.ml

+        | [] -> [if_case]
+        | [ G.E else_ ] ->
+          G.case_of_pat_and_expr
+            (G.PatLiteral (G.Null (G.fake "nil")), else_) :: if_case :: []


any reason why the else is tested first?

Because with improvements in tainting / constant propagation etc, the Null can be caught, while for the other case there is no test (except if we added a more explicit "not nil").

corneliuhoffman · 2025-12-29T11:24:38Z

languages/lisp/tree-sitter/Parse_clojure_tree_sitter.ml

+        | _ ->
+          G.PatId (param_id, G.empty_id_info ()))
+  in
+  (* What is the number of parameters in a pattern like #($_ ... %2 ...)?


a bit worried about the #($_ ... % ...) catching any number of params? Is that the case?

>= 1 params since % is %1

corneliuhoffman · 2025-12-29T12:15:12Z

languages/lisp/tree-sitter/Parse_clojure_tree_sitter.ml

+      let id_after = (after, t_after) in
+      GH.name_of_ids ?name_top (ids_before @ [ id_after ])
+  else
+    let should_add_colon =


bit of repetition here

corneliuhoffman · 2025-12-30T16:17:58Z

libs/ast_generic/AST_generic.ml

 (* ------------------------------------------------------------------------- *)

-let exprstmt e = s (ExprStmt (e, sc))
+let exprstmt e =


sorry why is this here?

It is something I thought I could optimise, but it did not work, and decided to document the situation...

corneliuhoffman · 2025-12-30T16:22:27Z

src/analyzing/AST_to_IL.ml

+      let arg_list_unwrapped =
+        List_.map (function
+            | G.Arg expr -> expr
+            | _ -> failwith "Expected G.Arg")


can this happen?

It should not happen.

corneliuhoffman · 2025-12-30T16:27:24Z

src/analyzing/AST_to_IL.ml

+                   (G.List, Tok.unsafe_fake_bracket arg_list_unwrapped)
+                 |> G.e) ]
+      in
+      call_generic env ~void tok eorig e (Tok.unsafe_fake_bracket arg_container)


so all functio calls have one argument which is a list?

yes.... this is because of the encoding where we have 1 implicit parameter which we pattern match on with Switch.

corneliuhoffman

more comments

corneliuhoffman · 2025-12-30T22:13:34Z

src/analyzing/Constant_propagation.ml

-    method! visit_resolved_name _env (_, sid) =
-      ok <- ok && ff sid;
-      if not ok then raise Exit
+    method! visit_resolved_name ff (_, sid) =


corneliuhoffman · 2025-12-30T22:14:12Z

src/analyzing/Constant_propagation.ml

+      if not (ff sid) then raise Exit
  end

+let no_cycles_in_sym_prop_visitor_instance = new no_cycles_in_sym_prop_visitor


any reason this is outside the function?

Of course! Why create an object each time if we can share one?

corneliuhoffman · 2025-12-30T22:14:37Z

src/analyzing/Visit_pattern_ids.ml

          super#visit_pattern store pat
  end

+let pat_id_visitor_instance = new pat_id_visitor


Some diagrams obtained from `-cfg_il` were impossible to read.

In one case a visitor was shared but had state, which is a bug.

The string is always lowercase because we convert it.

Also, sorted the list.

Reasons: - not used - not useful now or in the future

Be aware: this commit will not stay for long as is!

The `some->` translation is not accurate but better than nothing for now.

We now convert a pattern: ``` (-> e e1 ... e2) ``` not to: `(e2 (... (e1 e)))` but to: `(e2 (<... (e1 e) ...>))` which is closer to the intention. That is, the macroexpansion of the `->` requires this ellipsis to become deep ellipsis.

We can now match arbitrary sequences of test & expr pairs in cond-> / cond->>.

This allows to catch more intrafile / hof tainting cases.

This allows to capture more tainting; see test.

This is a small improvement: we don't always have to use ... ... to match a sequence of test-expr pairs.

We will later translate this properly.

The PR updating the grammar is now merged: opengrep/semgrep-clojure#1

This is because of keyboxd issues that suddenly appeared, will be fixed later.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [opengrep/opengrep](https://github.com/opengrep/opengrep) | minor | `v1.13.2` → `v1.14.1` | 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.14.1`](https://github.com/opengrep/opengrep/releases/tag/v1.14.1): Opengrep 1.14.1 [Compare Source](opengrep/opengrep@v1.14.0...v1.14.1) #### Improvements - Clojure translation part II by [@dimitris-m](https://github.com/dimitris-m) in [#517](opengrep/opengrep#517) - C#: Allow implicit variables in properties to be taint sources by [@maciejpirog](https://github.com/maciejpirog) in [#516](opengrep/opengrep#516) - Add core flags `dump_rule` and `dump_patterns_of_rule` as options in the show command by [@maciejpirog](https://github.com/maciejpirog) in [#519](opengrep/opengrep#519) #### Bug fixes - Fix: pass signature databaseb to lambda analysis, handle method mutation tainting by [@corneliuhoffman](https://github.com/corneliuhoffman) in [#520](opengrep/opengrep#520) #### Tech debt - Fix CHANGELOG.md, OPENGREP.md, remove unused files by [@dimitris-m](https://github.com/dimitris-m) in [#523](opengrep/opengrep#523) **Full Changelog**: <opengrep/opengrep@v1.14.0...v1.14.1> ### [`v1.14.0`](https://github.com/opengrep/opengrep/releases/tag/v1.14.0): Opengrep 1.14.0 [Compare Source](opengrep/opengrep@v1.13.2...v1.14.0) #### Improvements - Support for higher-order functions in intrafile taint analysis by [@corneliuhoffman](https://github.com/corneliuhoffman) in [#469](opengrep/opengrep#469) and [#513](opengrep/opengrep#513) - Clojure: Improved support for Clojure (incl. tainting) by [@dimitris-m](https://github.com/dimitris-m) in [#501](opengrep/opengrep#501) - Dart: Improved support for Dart by [@maciejpirog](https://github.com/maciejpirog) in [#508](opengrep/opengrep#508) - C#: Better handing of extension methods and extension blocks by [@maciejpirog](https://github.com/maciejpirog) in [#514](opengrep/opengrep#514) #### Fixes - Bump cygwin install action by [@dimitris-m](https://github.com/dimitris-m) in [#503](opengrep/opengrep#503) and [#509](opengrep/opengrep#509) **Full Changelog**: <opengrep/opengrep@v1.13.2...v1.14.0> </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 23, 2025 11:31

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

dimitris-m force-pushed the dm/unraw-clojure branch 7 times, most recently from d630851 to 289b1c5 Compare December 26, 2025 16:19

dimitris-m force-pushed the dm/unraw-clojure branch 2 times, most recently from ec3d9af to ae94211 Compare December 30, 2025 21:46

corneliuhoffman reviewed Dec 30, 2025

View reviewed changes

dimitris-m force-pushed the dm/unraw-clojure branch from c0fdc48 to 28b2e1c Compare December 30, 2025 23:20

dimitris-m added 11 commits December 31, 2025 01:14

increase ograph output to 300dpi

514a52d

Some diagrams obtained from `-cfg_il` were impossible to read.

reuse visitor instances, simplify

31fd8fc

In one case a visitor was shared but had state, which is a bug.

fix file type detection for dockerfiles

258dcbc

The string is always lowercase because we convert it.

fix comment indentation only

f13eb27

add clojure comment syntax for tests, spacegrep

7d7820c

fix lang_has_no_dollar_ids: clojure does

c61e579

Also, sorted the list.

fix syntax in clojure pattern tests

a122ae8

optimise stmt_to_expr for ExprStmt, add comments

b6350f4

remove unused recursive descent clj parser

de0946e

Reasons: - not used - not useful now or in the future

add debug log when we cannot print dataflow trace

97bb59f

remove XFAIL tag from docker test that now passes

aff38ad

dimitris-m force-pushed the dm/unraw-clojure branch from e1bdb78 to c34cce1 Compare December 31, 2025 01:14

dimitris-m added 14 commits December 31, 2025 05:09

clojure: translation with macroexpansion in IL

b3c2fb1

Be aware: this commit will not stay for long as is!

clojure: macroexpand threaded forms eagerly

70d5a6d

clojure: add some->, some->> and todos

c461643

The `some->` translation is not accurate but better than nothing for now.

clojure: fix deep ..., make (-> x ... e) match

31e986d

We now convert a pattern: ``` (-> e e1 ... e2) ``` not to: `(e2 (... (e1 e)))` but to: `(e2 (<... (e1 e) ...>))` which is closer to the intention. That is, the macroexpansion of the `->` requires this ellipsis to become deep ellipsis.

clojure: enable (cond-> e ... ... $TEST $FORM)

383b589

We can now match arbitrary sequences of test & expr pairs in cond-> / cond->>.

clojure: add when ... pattern test

7b4e59e

clojure: temporarily treat loop as call

d43798a

clojure: intrafile tainting tests and setup

c672623

clojure: improve processing of lambdas

700c05c

This allows to catch more intrafile / hof tainting cases.

clojure: add more pattern tests with macros

d69692e

clojure: process VarDef of lambda like FuncDef

1292449

This allows to capture more tainting; see test.

clojure: ensure that (cond-> $_E ...) works

9f9a740

This is a small improvement: we don't always have to use ... ... to match a sequence of test-expr pairs.

clojure: translate try-catch-finally

5332525

clojure: raise exception on parse error

5d48256

dimitris-m force-pushed the dm/unraw-clojure branch from 0517e24 to 5d48256 Compare December 31, 2025 05:11

dimitris-m changed the title ~~WIP: Clojure translation that supports tainting~~ Clojure translation that supports tainting Dec 31, 2025

dimitris-m added 2 commits December 31, 2025 05:19

clojure: don't fail on defmacro, make nil for now

b05ff06

We will later translate this properly.

clojure: update grammar submodule to opengrep/main

96bf24e

The PR updating the grammar is now merged: opengrep/semgrep-clojure#1

maciejpirog approved these changes Dec 31, 2025

View reviewed changes

dimitris-m force-pushed the dm/unraw-clojure branch 2 times, most recently from 755299c to 9a45ba5 Compare December 31, 2025 12:30

fix: disable OPAM sig verification in Win GHA

8c61f29

This is because of keyboxd issues that suddenly appeared, will be fixed later.

dimitris-m force-pushed the dm/unraw-clojure branch from 9a45ba5 to 8c61f29 Compare December 31, 2025 12:32

dimitris-m merged commit 1ad74d5 into main Dec 31, 2025
1 check passed

dimitris-m deleted the dm/unraw-clojure branch December 31, 2025 12:34

maciejpirog mentioned this pull request Dec 31, 2025

Release v1.14.0 #515

Merged

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

Conversation

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

Uh oh!

Pre-requisites

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dimitris-m commented Dec 23, 2025 •

edited

Loading

dimitris-m Dec 30, 2025 •

edited

Loading

dimitris-m Dec 30, 2025 •

edited

Loading