Semgrep: Metavariables don't match between class names and method return types in Java

Created on 2 Dec 2020  路  5Comments  路  Source: returntocorp/semgrep

Describe the bug
Semgrep doesn't seem to match type between class names and method return types for Java code.

I'm trying to identify race conditions in Java caused by multithreaded apps using singletons inappropriately. As part of that, I was trying to identify all singleton classes in a codebase; specifically all classes which have a static method which returns an instance of the class.

To Reproduce
https://semgrep.dev/s/Kd5P?stepByStep=1

Expected behavior
I would expect it to match the getInstance() method. When I change the pattern to pattern: public static $CLASS_2 $METHOD () { ... } it matches, but that's not what I'm looking for.

Priority
It's a blocker for my org's interest in semgrep.

bug external-user java high

All 5 comments

As I understand it, @mcbryde is wanting to enforce that the $CLASS metavariable in the two clauses references the same Java class.

I dug into this a bit.

In this match:

public class $CLASS { ... }

the metavariable $CLASS matches to a pure Name identifier node, whereas in:

public static $CLASS $VAR = ...

the metavariable $CLASS matches to a TyName node, and the metavariables don't match as a result.

Some thoughts on things we can do:

  • Change ClassDef so the the class name is a TyName; I worry that this has negative effects when matching the class as the target for static invocation
  • Change matching so that TyName nodes get bound to pure Name identifier nodes in metavariables
  • Extend more node data into the metavariable report in the core output JSON, and allow this matching in the Python wrapper

I'm personally a fan of option 2, but I'm not sure I have full insight into side effects of that change.

This now works on develop: https://semgrep.dev/s/QwAD/?version=develop

or like you did it originally: https://semgrep.dev/s/Kd5P?version=develop

@mcbryde this will ship out in our next release but for now it's in the trunk branch. Please let me know if you need a patch release cut or if you're ok waiting until next release early next week

Was this page helpful?
0 / 5 - 0 ratings