Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect pattern matching - When return object is a class with an import - Results are not correct #3748

Closed
1 task
tilperion2 opened this issue Aug 24, 2021 · 3 comments · Fixed by #4057
Closed
1 task
Labels
bug Something isn't working lang:java priority:medium user:external requested by someone outside of r2c

Comments

@tilperion2
Copy link

tilperion2 commented Aug 24, 2021

Describe the bug
Tested on of R2C reles(unrestricted-request-mapping) and found this issue:
When return class of method in pattern matching have class not as void or Java primitives, but class with import, then check is not working properly and False positive is reported. Look for 3rd reported line in example below.

I suppose in this case return class is not correctly checker/matched in $RETURNTYPE section of the rule.

Looks like reproduced only in lates v0.62.0

To Reproduce
https://semgrep.dev/s/nkx1

Expected behavior
Rule working correctly, imported classes in return statement are matched in a right way.
Rules with same $RETURNTYPE should not be in results, no FP should be reported.

Possible future improvement - Use classes with imports/real class names in semGrep.dev(https://semgrep.dev/r?q=unrestricted-request-mapping) and possibly in your tests in the code.

Screenshots
image

What is the priority of the bug to you?

  • P1: important to fix or quite annoying

Environment
Latest v0.62.0

@tilperion2 tilperion2 changed the title When return object is a class with an import - results are not correct Incorrect pattern matching - When return object is a class with an import - Results are not correct Aug 24, 2021
@IagoAbal IagoAbal added bug Something isn't working lang:java priority:medium user:external requested by someone outside of r2c labels Aug 25, 2021
@IagoAbal
Copy link
Collaborator

This seems to be the commit that introduced the bug (PR #3684):

commit 1fd2534dbe27a4b0db79f8c997815a03eab8a70d
Author: Yoann Padioleau <pad@returntocorp.com>
Date:   Fri Aug 6 14:48:15 2021 +0200

    Refactor m_name, factorize more code (#3684)

@tilperion2
Copy link
Author

Have found the same issue for OWASP Top 10 rule from SemGrep rulepack.
Problem still looks the same - when return object is imported class, it is incorrectly matched by pattern/pattern-not evaluation.

See example below:
https://semgrep.dev/s/Jxdj

@IagoAbal
Copy link
Collaborator

cc @aryx

aryx added a commit that referenced this issue Oct 13, 2021
This closes #3748

test plan:
test file included
aryx added a commit that referenced this issue Oct 13, 2021
ievans pushed a commit that referenced this issue Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lang:java priority:medium user:external requested by someone outside of r2c
Development

Successfully merging a pull request may close this issue.

2 participants