sonar-cryptography icon indicating copy to clipboard operation
sonar-cryptography copied to clipboard

Depending detection rules can lead to duplicate findings

Open n1ckl0sk0rtge opened this issue 1 year ago • 1 comments

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 avatar Jun 11 '24 12:06 n1ckl0sk0rtge

@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.

hugoqnc avatar Aug 16 '24 14:08 hugoqnc