semgrep / pfff

pfff is mainly an OCaml API to write static analysis, dynamic analysis, code visualizations, code navigations, or style-preserving source-to-source transformations such as refactorings on source code.
https://semgrep.dev
Other
186 stars 29 forks source link

allow equality checking for taint traces in parse_info #556

Closed brandonspark closed 2 years ago

brandonspark commented 2 years ago

What:

Currently, Parse_info.ts ignore the taint traces when comparing for equality.

This means, in particular, that later assumptions on this equality function make deduplication remove taint traces which have different call sites.

Why:

We should not do this, because we would like multiple findings from different call-sites, even if to the same sink (and from the same rule)

How:

Made the equality function properly derived.

Security

IagoAbal commented 2 years ago

Currently, Parse_info.ts ignore the taint traces when comparing for equality.

Where does Parse_info.t contain any taint trace?

aryx commented 2 years ago

It's a typo, he meant Pattern_match.t I think. He fixed in his semgrep PR, but still made the mistake for the pfff PR.

IagoAbal commented 2 years ago

It's a typo, he meant Pattern_match.t I think. He fixed in his semgrep PR, but still made the mistake for the pfff PR.

But Pattern_match.t is not in Pfff. I tried to find a corresponding PR in Semgrep. We should not be using taint traces for deduplication unless we fix the CLI to take taint traces and --datafow-traces into account. Otherwise, from the user perspective, we will be reporting duplicate findings. (update: OK I found the Semgrep PR.)

aryx commented 2 years ago

I've hesitated also to merge this PR, because I feel you should never use = on a Parse_info. But it's a core parsing library so you never know, so I've merged it.