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

Depending detection rules can lead to duplicate findings #12

Closed
n1ckl0sk0rtge opened this issue Jun 11, 2024 · 1 comment · Fixed by #142
Closed

Depending detection rules can lead to duplicate findings #12

n1ckl0sk0rtge opened this issue Jun 11, 2024 · 1 comment · Fixed by #142
Assignees
Labels
bug Something isn't working

Comments

@n1ckl0sk0rtge
Copy link
Member

n1ckl0sk0rtge commented Jun 11, 2024

Let's look at the example below, where our rule NEW_CIPHER (actual rule from Python's CryptographyCipher.java file) has two parameters that need to be detected:

private static final IDetectionRule<Tree> NEW_CIPHER = new DetectionRuleBuilder<Tree>()
    .createDetectionRule()
    .forObjectTypes("cryptography.hazmat.primitives.ciphers")
    .forMethods("Cipher")
    .withMethodParameter("cryptography.hazmat.primitives.ciphers.algorithms.*")
        .shouldBeDetectedAs(new AlgorithmFactory())
    .withMethodParameter("cryptography.hazmat.primitives.ciphers.modes.*")
        .shouldBeDetectedAs(new AlgorithmFactory())
    .buildForContext(new CipherContext())
    .withDependingDetectionRules(followingNewCipherRules);

This rule has a set of depending detection rules followingNewCipherRules at the level of the method (and not related to a parameter).
With the current handling of depending detection rules, once a parameter is detected, the function analyseExpression is called, which will call onReceivingNewDetection.
Here, both "method parameter related detection rules" and "invoked object related detection rules" are handled using followNextRules.
In our particular case, onReceivingNewDetection will be called twice, once for each parameter, which will therefore duplicate "invoked object related detection rules" and their related findings.

Therefore, calling "invoked object related detection rules" should not be done at this point. This may be fixed in the more general refactoring of entry points described in issue #9 or in the more general refactoring of subrules handling described in issue #8.

@n1ckl0sk0rtge n1ckl0sk0rtge added the bug Something isn't working label Jun 11, 2024
@hugoqnc
Copy link
Member

hugoqnc commented Aug 16, 2024

@n1ckl0sk0rtge
This bug of duplicated findings also appears when a detection rule has a top level detection, a parameter detection and a global depending detection rule.
The test DuplicateDependingRules2Test.java showcases the bug on a simple rule, independent of the cryptography rules.

n1ckl0sk0rtge added a commit that referenced this issue Sep 11, 2024
Signed-off-by: Nicklas Körtge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants