IBM / sonar-cryptography

This repository contains a SonarQube Plugin that detects cryptographic assets in source code and generates CBOM.
Apache License 2.0
23 stars 4 forks source link

Depending detection rules can lead to duplicate findings #12

Closed n1ckl0sk0rtge closed 2 weeks ago

n1ckl0sk0rtge commented 3 months ago

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.

hugoqnc commented 1 month ago

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