How to resolve rules "Cypher Block Chaining IV's should be random and unique"

SonarQube Server / Community Build
,
1

Hi all,

I am using SonarQube 6.7.5 with java plugin 5.7 (build 15470)

I analyze these source code with SonarLint Eclipse:

import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Arrays;

import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.SecretKey;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.KeyGenerator;

/**
 * 
 * @author developer
 * source code taken from https://wiki.sei.cmu.edu/confluence/display/java/MSC61-J.+Do+not+use+insecure+or+weak+cryptographic+algorithms
 */
class Msc61 {
    public static SecretKey generateKey() {
        try {
            KeyGenerator kgen = KeyGenerator.getInstance("AES");
            kgen.init(128);
            return kgen.generateKey();
        } catch (NoSuchAlgorithmException e) {
            throw new IllegalStateException(e.toString());
        }
    }
 
    public static byte[] encrypt(SecretKey skey, String plaintext) {
        /* Precond: skey is valid; otherwise IllegalStateException will be thrown. */
        try {
            byte[] ciphertext = null;
            Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");           
            final int blockSize = cipher.getBlockSize();
            byte[] initVector = new byte[blockSize];
            (new SecureRandom()).nextBytes(initVector);
            IvParameterSpec ivSpec = new IvParameterSpec(initVector);
            cipher.init(Cipher.ENCRYPT_MODE, skey, ivSpec);
            byte[] encoded = plaintext.getBytes(java.nio.charset.StandardCharsets.UTF_8);
            ciphertext = new byte[initVector.length + cipher.getOutputSize(encoded.length)];
            for (int i=0; i < initVector.length; i++) {
                ciphertext[i] = initVector[i];
            }
            // Perform encryption
            cipher.doFinal(encoded, 0, encoded.length, ciphertext, initVector.length);
            return ciphertext;
        } catch (NoSuchPaddingException | InvalidAlgorithmParameterException | ShortBufferException |
            BadPaddingException | IllegalBlockSizeException | InvalidKeyException | NoSuchAlgorithmException e)
        {
            /* None of these exceptions should be possible if precond is met. */
            throw new IllegalStateException(e.toString());
        }
    }
 
    public static String decrypt(SecretKey skey, byte[] ciphertext)
        throws BadPaddingException, IllegalBlockSizeException /* these indicate corrupt or malicious ciphertext */
    {
        try {
            Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");           
            final int blockSize = cipher.getBlockSize();
            byte[] initVector = Arrays.copyOfRange(ciphertext, 0, blockSize);
            IvParameterSpec ivSpec = new IvParameterSpec(initVector);
            cipher.init(Cipher.DECRYPT_MODE, skey, ivSpec);
            byte[] plaintext = cipher.doFinal(ciphertext, blockSize, ciphertext.length - blockSize);
            return new String(plaintext);
        } catch (NoSuchPaddingException | InvalidAlgorithmParameterException |
            InvalidKeyException | NoSuchAlgorithmException e)
        {
            /* None of these exceptions should be possible if precond is met. */
            throw new IllegalStateException(e.toString());
        }
    }
}

On December, I analyze them with SonarLint Eclipse 3.x and there are no issues “Cypher Block Chaining IV’s should be random and unique” .
Today, I analyze them again with SonarLint Eclipse 4.0 without changing any codes and I got issues “Cypher Block Chaining IV’s should be random and unique” in method decrypt.

How to resolve this issues?

Thanks :smiley:

2

Hi,

Your upgrade of SonarLint also upgraded the embedded analyzers. That’s why you’re benefiting from new rules. As for how to solve this, have you checked out the rule description?

 
:slight_smile:
Ann

3

Is embedded analyzers that you mentioned is Sonar-java plugin?

I have read the description and resolved this rules before by using the codes I share, and it was closed. But after I upgrade SonarLint to 4 (before was 3) on Eclipse, it is reopened.

5

Any updates for this issues?

6

Hi @fanny_tan

I think we should forget about SonarLint here and just understand the current state of analysis.

In the code snippet you provided, there are 2 new IvParameterSpec(...), in encrypt and decrypt.

In encrypt you are using a secure way to init initVector: (new SecureRandom()).nextBytes(initVector);, so no issue here.

In decrypt you are relying on the method parameter ciphertext. I think, it’s not safe here, so IMO the issue you are getting here is a good one.

Coming back to SonarLint, I don’t know why issue(s) disappeared/appeared, there could be many reasons, and looks like it’s impossible to investigate them now.

Lena

7

@Lena Thank you.

Right now I think I will try to fix the issues. Do you have an example of how to fixed this decrypt issues?

8

This is a tricky one :
The rule is implemented rather naively and not relying on the context where the IV is used. When used in decrypt it is safe to not randomly initialized it.

So, contrary to what @Lena mentioned (sorry mate :wink: ) it is safe in your case. The rule just check that when you construct an IV in a method, some secureRandom call is made in that method.

It should be improved with dataflow analysis at some point, in the meantime you can mark the issue as false positive.
Thanks for the feedback.

1 Like
9

Thank you so much @Nicolas_Peru :smile: