False Negative: Weak Algorithm Spec with Different Case to be used with Cipher is not reported

Rules and Languages Report False-positive / False-negative...
, ,
1

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

package com.minimals.Cipher.stringCaseTransform;
import java.security.NoSuchAlgorithmException;
import java.util.Locale;

import javax.crypto.Cipher;
import javax.crypto.NoSuchPaddingException;

public class CipherExample  {
    public static void main(String[] args) 
    throws NoSuchAlgorithmException, NoSuchPaddingException {
        Cipher c = Cipher.getInstance("des".toUpperCase(Locale.ENGLISH));
        System.out.println(c.toString());
      }
}

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 or des are used directly, such as:

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

and

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!

SonarQube 9.6 is officially EOL. Can you upgrade to the Latest / current LTS: 9.9, and see if this is still replicable?

 
Thx,
Ann

3

Hi Ann!

Thank you for getting back to me and welcome to the community! 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. I will do the same for the other two posts.

Thanks,
Scott

Screen Shot 2023-03-24 at 4.53.26 PM
Screen Shot 2023-03-24 at 4.53.26 PM2014×1286 188 KB

Screen Shot 2023-03-24 at 5.05.22 PM
Screen Shot 2023-03-24 at 5.05.22 PM2534×1158 256 KB

6

Hey Scott, thank you for the report! Currently the check that is looking for this kind of issues does not try to evaluate method invocations: that means that it will only raise issues on direct literal strings passed to the getInstance or on identifiers that point to constants like

private static final String alg = "DES"

To fix these types of FN would be a very nice improvement, also because it would benefit other rules relying on string value resolutions. I have a created a ticket to be aware of this, hopefully we will be able to improve this soon!

Have a nice day