False positive or false negative? I don't care which but consistency would be real nice as a minimum

Here’s another one of these infuriating cases where the tool can’t make up its mind.

I get a “critical” security warning on this:

    Cipher c = Cipher.getInstance("AES/CBC/PKCS5Padding");

That sounds legit to me.

But then, I find another line in a class right next to this one, with no warning at all:

    Cipher c = Cipher.getInstance("AES/CBC/PKCS5Padding", "BC");

Both are using the same algorithm which is surely insecure. Questions:

  • Is this algorithm bad in some situations and not in others? i.e., is Sonar trying to detect how we’re using the cipher after getting it? Or,
  • Is this algorithm only bad if the provider is not Java’s default one? i.e., is it something about the way the algorithm is implemented in Java which is bad, and alternative providers might have an implementation which is not vulnerable? Or,
  • Is Sonar erroneously only complaining about one overload of the method? i.e., is one of the two a false positive, or a false negative?

Hi @trejkaz

Thank for your feedback.
Can you confirm this is the rule that reports an issue on your first code snippet?
https://rules.sonarsource.com/java/RSPEC-5542

That’s the one. Notably there are no examples of the two-arg version in the good/bad examples either.

Hi,

From the rule description:

Cipher Block Chaining (CBC) with PKCS#5 padding (or PKCS#7) is vulnerable to padding oracle attacks.

Both examples are using CBC mode with padding so both are vulnerable.

For some reason I am not able to reproduce the inconsistency. I analyzed the same two lines assuming that you import javax.crypto.Cipher.

Is the project you analyze open-source? Is yes could you point to the project file in a public repository?
If not, can you share a code snippet or file in private?

Thank you.