False Negative: Weak Algorithm Spec from Object to be used with Cipher is not reported

Clean Code Report False-positive / False-negative...
, ,
1

We ran SonarQube with Java to scan the following code snippet:

package com.minimals.Cipher.KeyToAlgoDES;
import javax.crypto.Cipher;
import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
public class CipherExample {
        public static void main(String[] args) {
        try{
            String algo = "DES";
            KeyGenerator keygen = KeyGenerator.getInstance(algo);
            SecretKey key = keygen.generateKey();
            System.out.println(key.getAlgorithm());
            Cipher c = Cipher.getInstance(keygen.getAlgorithm());
            // Cipher c = Cipher.getInstance(key.getAlgorithm().toUpperCase());
            System.out.println(c.getAlgorithm() + " " + key.getAlgorithm());
            c.init(Cipher.ENCRYPT_MODE, key);
            c.doFinal("something".getBytes());
        } catch(Exception e ){
            e.printStackTrace();
        }
    }
}

We scanned the source Java file using the docker. The command we used is:

sonar-scanner \
  -Dsonar.projectKey={PROJECT NAME} \
  -Dsonar.sources=. \
  -Dsonar.host.url=http://localhost:9000 \
  -Dsonar.login={PROJECT KEY}

However, while it is possible to statically compute the value to be passed to Cipher.getInstance(), SonarQube does not report it.

Of course, SonarQube reports the issue when DES is used directly, such as:

Cipher c = Cipher.getInstance("DES");

in a similar source file.

Which is why I think it is a false negative.

Additional Details

Language: Java
Rule: java:S5547, java:S5542
Product Details:
SonarQube Community Edition Version - 9.6.1
Sonar Scanner Version - 4.7
Java Version - Java 11.0.14.1 Eclipse Adoptium (64-bit)
Operating System - MacOS Monterey version 12.6

2

Hi,

Welcome to the community and thanks for this report!

Can you upgrade to the current version, 9.9, and see if this is still reproducable?

 
Thx,
Ann

3

Hi Ann!

Thank you for the warm welcome and for SonarQube! I have upgraded the SonarQube version to 9.9.0.65466 and can confirm that the issue still persists and is reproducible. I have also attached screenshots of SonarQube results and docker details here for your convenience.

image
image2528×1302 293 KB

image
image2014×1286 154 KB

7

Hey @Amit_Seal_Ami,
Thanks for reporting this issue.
Due to some limitations discussed in this ticket, S5547 does not look into the result of method calls (and I am not sure that we want to resolve expressions that include variables whose initialization we need to inspect).

However, because KeyGenerator is part of the standard library, we already have rules that cover the use of weak ciphers for key generation (eg S4426).

Does SonarQube raise other security issues on the snippet of code you submitted?

Cheers,

Dorian

8

Dear Dorian_Burihabwa,

Thank you for the response!

SonarQube does not report any other warnings. All the warnings counts are 0, when we checked.

To provide more context about what I am about to ask, my team is conducting academic research on Java Cryptographic API-related misuse. As you and Leonardo_Pilastri mentioned, there are several design decisions and philosophies in SonarQube that results in the false negatives I reported:

  1. The analyzer avoids evaluating non-constant identifiers, even if they are statically determinable, which is motivated by,
  2. SonarQube’s priority to not raise FPs.

Based on these, we have the following questions:

  1. What factors motivated SonarQube to prioritize not raising FPs in this specific context, i.e., by not analyzing for non-constant identifiers?
  2. Do you prioritize not raising FP for all four Clean Code attributes (Quality, Security, Maintainability, and Reliability) equally, or are there variations? For example, in this case, you mentioned that analyzing non-constant identifiers may raise FPs, but it is still worth investigating to reduce false negatives. Is this somewhat of an exception because it is related to security?
  3. Are there any additional factors you consider when prioritizing the trade-off between FP and FN?
  4. What’s the threshold SonarQube aims for when it comes to maintaining the percentage of FP, if there is any (even if informally)?

Please let us know what you think. Thanks again for your very valuable insight!