Fixed top level HOFS and inlined lambda by corneliuhoffman · Pull Request #513 · opengrep/opengrep

corneliuhoffman · 2025-12-30T13:08:11Z

Two issues with HOF taint analysis were not working:

Inline lambda calls: Calling a lambda immediately after definition wasn't tracked

 (lambda x: sink(x))(source())  # Not detected

Top-level HOF callbacks: Lambdas/functions defined at module level and passed to HOFs weren't being analyzed

toplevel_sink = ->(x) { sink(x) }
toplevel_items.each(&toplevel_sink)  # Not detected

Solution

Inline lambda calls (Dataflow_tainting.ml)

Detect lambda expressions in callee position and analyze them inline with tainted arguments

Top-level HOF callbacks (Function_call_graph.ml, Visit_function_defs.ml)

Add visit_with_parent_path to track parent function context during AST traversal
Extract HOF callbacks from top-level code (outside function bodies)
Add edges from top-level lambdas to <top_level> node in call graph

Ruby/PHP block parameters (AST_to_IL.ml)

Handle &callback syntax (OtherParam("Ref", ...)) as proper parameters for taint tracking

Builtin models (Builtin_models.ml)

Add MethodHOF for Ruby to handle .each(&callback) pattern

Code cleanup

Simplified constructor implicit edge logic (removed unnecessary exclusion)
Removed unsafe fallback matching ANY class method by name
Commented out debug dot file generation

Test plan

All HOF comprehensive tests pass (Ruby, Scala, Python, JS, etc.)
New test: tests/tainting/lambda_inline_call.py
New test: tests/tainting/toplevel_hof_callback.py

A bug was introduced in using AST_toIL.name_of_entity. THat forces some sid's and naming that messes up the equalities in func_id. this fixes that

dimitris-m · 2025-12-30T22:12:03Z

src/analyzing/Visit_function_defs.ml

+          let class_il = Option.bind !current_class g_name_to_il_name in
+          let func_il = entity_to_il_name ent in


But AST_to_IL.var_of_name also deals with IdQualified.

Is there no reason to deal with that in g_name_to_il_name?

I ampretty sure not (we only use it for classes and methods)

dimitris-m · 2025-12-30T22:16:23Z

src/analyzing/Visit_function_defs.ml

+(* Visit all Call expressions at top level (outside function bodies).
+   Callback receives: full call expr, callee expr, and arguments *)
+let visit_toplevel_calls (f : G.expr -> G.expr -> G.arguments -> unit) (ast : G.program) : unit =
+  let v = new toplevel_call_visitor in


Is there a reason to create a new instance on each call?

Why not share the visitor (pull out of function)?

oh ... that answers my question in your PR :)))
My point is that I wanted the function to be kindda pure ... despirte the visitor mess inside. And that way I don't have to worry too much about threards

dimitris-m · 2025-12-30T22:16:44Z

src/analyzing/Visit_function_defs.ml

+let fold_toplevel_calls (f : 'acc -> G.expr -> G.expr -> G.arguments -> 'acc)
+    (init_acc : 'acc) (ast : G.program) : 'acc =
+  let acc_ref = ref init_acc in
+  let v = new toplevel_call_visitor in


As above, visitor instance can be shared.

as above, can change but did not want to think about threads

tests/rules/cross_function_tainting/test_hof_comprehensive_c.c

dimitris-m · 2025-12-30T22:42:25Z

src/tainting/Function_call_graph.ml

+        (match e.G.e with
+        | G.Call (callee, args) ->
+            let found = extract_hof_callbacks_from_call
+              ~method_hofs ~function_hofs ~user_hofs ~all_funcs ~caller_parent_path
+              callee args
+            in
+            callbacks := found @ !callbacks
+        | _ -> ());
+        super#visit_expr env e


Leaving as TODO to extract this into a class, there are some potential concurrency issues with objects + domains.
It's one of the reasons I converted most (all?) such objects into classes and had the instances stateless & shared.

dimitris-m

LGTM

Some small comments you may consider.

We should close #500 once merged.

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).

corneliuhoffman requested review from dimitris-m, maciejpirog and willem-delbare as code owners December 30, 2025 13:08

Corneliu Hoffman added 2 commits December 30, 2025 18:59

fixed top level call and inlined lambda

971b39a

fixed name to IL.name bug in Visit_functions.

a84340a

A bug was introduced in using AST_toIL.name_of_entity. THat forces some sid's and naming that messes up the equalities in func_id. this fixes that

corneliuhoffman force-pushed the HOF_fixes branch from 78ce104 to a84340a Compare December 30, 2025 19:00

dimitris-m removed the request for review from willem-delbare December 30, 2025 22:04

dimitris-m reviewed Dec 30, 2025

View reviewed changes

tests/rules/cross_function_tainting/test_hof_comprehensive_c.c Show resolved Hide resolved

dimitris-m reviewed Dec 30, 2025

View reviewed changes

dimitris-m approved these changes Dec 30, 2025

View reviewed changes

maciejpirog merged commit 920e6c2 into main Dec 30, 2025
6 checks passed

maciejpirog deleted the HOF_fixes branch December 30, 2025 23:05

maciejpirog mentioned this pull request Dec 31, 2025

Release v1.14.0 #515

Merged

dimitris-m added the taint label Jan 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed top level HOFS and inlined lambda#513

Fixed top level HOFS and inlined lambda#513
maciejpirog merged 2 commits intomainfrom
HOF_fixes

corneliuhoffman commented Dec 30, 2025

Uh oh!

dimitris-m Dec 30, 2025

Uh oh!

corneliuhoffman Dec 30, 2025

Uh oh!

dimitris-m Dec 30, 2025

Uh oh!

corneliuhoffman Dec 30, 2025

Uh oh!

dimitris-m Dec 30, 2025

Uh oh!

corneliuhoffman Dec 30, 2025

Uh oh!

Uh oh!

dimitris-m Dec 30, 2025

Uh oh!

dimitris-m left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let class_il = Option.bind !current_class g_name_to_il_name in
		let func_il = entity_to_il_name ent in

Conversation

corneliuhoffman commented Dec 30, 2025

Uh oh!

dimitris-m Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

dimitris-m Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

dimitris-m Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

corneliuhoffman Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimitris-m Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

dimitris-m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants