"new" keyword in C# throws error

[jagprog5]

semgrep version v1.70.0

rules:
  - id: id-here
    languages:
      - csharp
    severity: WARNING
    message: HashAlgorithm $NAME detected
    patterns:
      - pattern: new System.Security.Cryptography.HashAlgorithmName("$NAME")
      - metavariable-pattern:
          metavariable: $NAME
          pattern-either:
            - pattern: SHA3_384
            - pattern: SHA3_256
            - pattern: SHA384
            - pattern: SHA256
            - pattern: SHA1
            - pattern: MD5
            - pattern: SHA3_512
            - pattern: SHA512

test code:

using System.Security.Cryptography;

class Program {
    static void Main() {
        HashAlgorithmName name = new HashAlgorithmName("SHA1");
    }
}

Adding the word "new" in the above rule causes the following error:

Internal matching error when running hashname-standard-lib-c-sharp on target.cs:
 An error occurred while invoking the Semgrep engine. Please help us fix this by creating an issue at https://github.com/returntocorp/semgrep

metavariable-pattern failed because $NAME does not bind to a sub-program, please check your rule

My workaround is to replace the above metavariable-pattern with an equivalent metavariable-regex.

What is the priority of the bug to you?

• P0: blocking your adoption of Semgrep or workflow
• P1: important to fix or quite annoying
• P2: regular bug that should get fixed

Environment
Semgrep playground

[amchiclet]

Reduced the test. The issue seems to be language-agnostic.

# python code
f("some-name")

This rule causes the error: metavariable-pattern failed because $NAME does not bind to a sub-program, please check your rule

rules:
  - id: some-id
    languages:
      - python
    severity: WARNING
    message: some-message
    patterns:
      - pattern: f("$NAME")
      - metavariable-pattern:
          metavariable: $NAME
          pattern: some-name

[liukatkat]

The root cause of the error is due to the way we match metavariables inside quotes. Metavariables in quotes are handled differently than those not in quotes. When they are enclosed in strings, you should use metavariable-regex to check its contents. We will improve our documentation to call out this distinction!

The reason why the error is thrown only after you added the new keyword is because without the new keyword, it would not match HashAlgorithmName("SHA1");, which is checked before we try to match the metavariables. We are looking into why that is the case.

[liukatkat]

The error aside, I looked into why the rule without the new keyword would not match HashAlgorithmName("SHA1") and here's what I found:

Based on our current implementation, functions don't match constructors. Without the new keyword, pattern: System.Security.Cryptography.HashAlgorithmName("$NAME") is parsed as a function and would not match HashAlgorithmName("SHA1"), which is (part of) a constructor. Therefore, we will not perform the comparison related to the metavariable (and no error is thrown).

We have decided that it is better to keep the distinction between functions and constructors whenever we can disambiguate the two, as constructors are hard to deal with when certain languages have numerous ways of constructing objects. It seems like your workaround works fine so I will close this issue with this comment.