From b9f403a2655f7dee2a7ec017e3bba57ad3d98cbd Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 22 Oct 2020 11:43:18 +0200 Subject: [PATCH] Fixed possible integer overflows and implicit casting The issues were reported by LGTM.com --- .../org/bouncycastle/crypto/digests/SkeinEngine.java | 2 +- .../generators/GOST3410ParametersGenerator.java | 8 ++++---- .../bouncycastle/crypto/io/CipherInputStream.java | 6 +++++- .../main/java/org/bouncycastle/crypto/macs/KMAC.java | 6 +++--- .../crypto/signers/ISO9796d2PSSSigner.java | 4 ++-- .../bouncycastle/math/ec/rfc7748/X25519Field.java | 2 +- .../pqc/crypto/gmss/util/WinternitzOTSVerify.java | 6 +++--- .../pqc/crypto/gmss/util/WinternitzOTSignature.java | 6 +++--- .../org/bouncycastle/pqc/crypto/qtesla/QTesla1p.java | 4 ++-- .../org/bouncycastle/pqc/crypto/qtesla/QTesla3p.java | 12 ++++++------ .../org/bouncycastle/pqc/crypto/sphincs/Horst.java | 2 +- .../pqc/math/ntru/polynomial/IntegerPolynomial.java | 4 ++-- 12 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/bouncycastle/crypto/digests/SkeinEngine.java b/core/src/main/java/org/bouncycastle/crypto/digests/SkeinEngine.java index 1d4e84093c..e455e7b0c3 100644 --- a/core/src/main/java/org/bouncycastle/crypto/digests/SkeinEngine.java +++ b/core/src/main/java/org/bouncycastle/crypto/digests/SkeinEngine.java @@ -689,7 +689,7 @@ private void createInitialState() } // Process configuration block - ubiComplete(PARAM_TYPE_CONFIG, new Configuration(outputSizeBytes * 8).getBytes()); + ubiComplete(PARAM_TYPE_CONFIG, new Configuration(outputSizeBytes * 8L).getBytes()); } // Process additional pre-message parameters diff --git a/core/src/main/java/org/bouncycastle/crypto/generators/GOST3410ParametersGenerator.java b/core/src/main/java/org/bouncycastle/crypto/generators/GOST3410ParametersGenerator.java index 8383175c05..bbe61f983e 100644 --- a/core/src/main/java/org/bouncycastle/crypto/generators/GOST3410ParametersGenerator.java +++ b/core/src/main/java/org/bouncycastle/crypto/generators/GOST3410ParametersGenerator.java @@ -162,12 +162,12 @@ private long procedure_Aa(long x0, long c, BigInteger[] pq, int size) //Verify and perform condition: 04294967296L) { - x0 = init_random.nextInt()*2; + x0 = init_random.nextInt()*2L; } while((c<0 || c>4294967296L) || (c/2==0)) { - c = init_random.nextInt()*2+1; + c = init_random.nextInt()*2L+1L; } BigInteger C = new BigInteger(Long.toString(c)); @@ -371,12 +371,12 @@ private void procedure_Bb(long x0, long c, BigInteger[] pq) //Verify and perform condition: 04294967296L) { - x0 = init_random.nextInt()*2; + x0 = init_random.nextInt()*2L; } while((c<0 || c>4294967296L) || (c/2==0)) { - c = init_random.nextInt()*2+1; + c = init_random.nextInt()*2L+1; } BigInteger [] qp = new BigInteger[2]; diff --git a/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java b/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java index b06d1f53e0..80eb770810 100644 --- a/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java +++ b/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java @@ -298,7 +298,11 @@ public long skip( int avail = available(); if (n <= avail) { - bufOff += n; + if (bufOff > Integer.MAX_VALUE - n) + { + throw new IOException("Unable to skip: integer overflow"); + } + bufOff += n; // lgtm [java/implicit-cast-in-compound-assignment] return n; } diff --git a/core/src/main/java/org/bouncycastle/crypto/macs/KMAC.java b/core/src/main/java/org/bouncycastle/crypto/macs/KMAC.java index 8bdc9f4d00..8e5cd05f6f 100644 --- a/core/src/main/java/org/bouncycastle/crypto/macs/KMAC.java +++ b/core/src/main/java/org/bouncycastle/crypto/macs/KMAC.java @@ -105,7 +105,7 @@ public int doFinal(byte[] out, int outOff) throw new IllegalStateException("KMAC not initialized"); } - byte[] encOut = XofUtils.rightEncode(getMacSize() * 8); + byte[] encOut = XofUtils.rightEncode(getMacSize() * 8L); cshake.update(encOut, 0, encOut.length); } @@ -126,7 +126,7 @@ public int doFinal(byte[] out, int outOff, int outLen) throw new IllegalStateException("KMAC not initialized"); } - byte[] encOut = XofUtils.rightEncode(outLen * 8); + byte[] encOut = XofUtils.rightEncode(outLen * 8L); cshake.update(encOut, 0, encOut.length); } @@ -199,6 +199,6 @@ private void bytePad(byte[] X, int w) private static byte[] encode(byte[] X) { - return Arrays.concatenate(XofUtils.leftEncode(X.length * 8), X); + return Arrays.concatenate(XofUtils.leftEncode(X.length * 8L), X); } } diff --git a/core/src/main/java/org/bouncycastle/crypto/signers/ISO9796d2PSSSigner.java b/core/src/main/java/org/bouncycastle/crypto/signers/ISO9796d2PSSSigner.java index 2ee1808861..80f83381d3 100644 --- a/core/src/main/java/org/bouncycastle/crypto/signers/ISO9796d2PSSSigner.java +++ b/core/src/main/java/org/bouncycastle/crypto/signers/ISO9796d2PSSSigner.java @@ -394,7 +394,7 @@ public byte[] generateSignature() digest.doFinal(m2Hash, 0); byte[] C = new byte[8]; - LtoOSP(messageLength * 8, C); + LtoOSP(messageLength * 8L, C); digest.update(C, 0, C.length); @@ -514,7 +514,7 @@ public boolean verifySignature( // check the hashes // byte[] C = new byte[8]; - LtoOSP(recoveredMessage.length * 8, C); + LtoOSP(recoveredMessage.length * 8L, C); digest.update(C, 0, C.length); diff --git a/core/src/main/java/org/bouncycastle/math/ec/rfc7748/X25519Field.java b/core/src/main/java/org/bouncycastle/math/ec/rfc7748/X25519Field.java index f315a1fa46..333e458170 100644 --- a/core/src/main/java/org/bouncycastle/math/ec/rfc7748/X25519Field.java +++ b/core/src/main/java/org/bouncycastle/math/ec/rfc7748/X25519Field.java @@ -503,7 +503,7 @@ private static void reduce(int[] z, int x) int t = z[9], z9 = t & M24; t = (t >> 24) + x; - long cc = t * 19; + long cc = t * 19L; cc += z[0]; z[0] = (int)cc & M26; cc >>= 26; cc += z[1]; z[1] = (int)cc & M26; cc >>= 26; cc += z[2]; z[2] = (int)cc & M25; cc >>= 25; diff --git a/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSVerify.java b/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSVerify.java index 4c2e501099..72162db463 100644 --- a/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSVerify.java +++ b/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSVerify.java @@ -77,9 +77,9 @@ public byte[] Verify(byte[] message, byte[] signature) byte[] testKey = new byte[testKeySize]; - int c = 0; + long c = 0; int counter = 0; - int test; + long test; if (8 % w == 0) { @@ -279,7 +279,7 @@ public int getLog(int intValue) return log; } - private void hashSignatureBlock(byte[] sig, int sigOff, int rounds, byte[] buf, int bufOff) + private void hashSignatureBlock(byte[] sig, int sigOff, long rounds, byte[] buf, int bufOff) { if (rounds < 1) { diff --git a/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSignature.java b/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSignature.java index 931ed4872c..80686e9363 100644 --- a/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSignature.java +++ b/core/src/main/java/org/bouncycastle/pqc/crypto/gmss/util/WinternitzOTSignature.java @@ -131,8 +131,8 @@ public byte[] getSignature(byte[] message) // byte [] message; // message m as input byte[] hash = new byte[mdsize]; // hash of message m int counter = 0; - int c = 0; - int test = 0; + long c = 0; + long test = 0; // create hash of message m messDigestOTS.update(message, 0, message.length); messDigestOTS.doFinal(hash, 0); @@ -326,7 +326,7 @@ public int getLog(int intValue) return log; } - private void hashPrivateKeyBlock(int index, int rounds, byte[] buf, int off) + private void hashPrivateKeyBlock(int index, long rounds, byte[] buf, int off) { if (rounds < 1) { diff --git a/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla1p.java b/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla1p.java index e33c6f9fe7..8a4c1d85f7 100644 --- a/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla1p.java +++ b/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla1p.java @@ -1194,11 +1194,11 @@ static void sparse_mul32(int[] prod, int ppos, int[] pk, int pkPos, int[] pos_li pos = pos_list[i]; for (j = 0; j < pos; j++) { - temp[j] = temp[j] - sign_list[i] * pk[pkPos + j + PARAM_N - pos]; + temp[j] = temp[j] - (long) sign_list[i] * pk[pkPos + j + PARAM_N - pos]; } for (j = pos; j < PARAM_N; j++) { - temp[j] = temp[j] + sign_list[i] * pk[pkPos + j - pos]; + temp[j] = temp[j] + (long) sign_list[i] * pk[pkPos + j - pos]; } } diff --git a/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla3p.java b/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla3p.java index 7a8ce3ca6b..b8bae68582 100644 --- a/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla3p.java +++ b/core/src/main/java/org/bouncycastle/pqc/crypto/qtesla/QTesla3p.java @@ -1392,11 +1392,11 @@ static void sparse_mul8(long[] prod, int ppos, byte[] s, int spos, int[] pos_lis pos = pos_list[i]; for (j = 0; j < pos; j++) { - prod[ppos + j] = prod[ppos + j] - sign_list[i] * s[spos + j + PARAM_N - pos]; + prod[ppos + j] = prod[ppos + j] - (long) sign_list[i] * s[spos + j + PARAM_N - pos]; } for (j = pos; j < PARAM_N; j++) { - prod[ppos + j] = prod[ppos + j] + sign_list[i] * s[spos + j - pos]; + prod[ppos + j] = prod[ppos + j] + (long) sign_list[i] * s[spos + j - pos]; } } } @@ -1417,11 +1417,11 @@ static void sparse_mul8(long[] prod, byte[] s, int[] pos_list, short[] sign_list pos = pos_list[i]; for (j = 0; j < pos; j++) { - prod[j] = prod[j] - sign_list[i] * t[j + PARAM_N - pos]; + prod[j] = prod[j] - (long) sign_list[i] * t[j + PARAM_N - pos]; } for (j = pos; j < PARAM_N; j++) { - prod[j] = prod[j] + sign_list[i] * t[j - pos]; + prod[j] = prod[j] + (long) sign_list[i] * t[j - pos]; } } } @@ -1489,11 +1489,11 @@ static void sparse_mul32(long[] prod, int ppos, int[] pk, int pkPos, int[] pos_l pos = pos_list[i]; for (j = 0; j < pos; j++) { - prod[ppos + j] = prod[ppos + j] - sign_list[i] * pk[pkPos + j + PARAM_N - pos]; + prod[ppos + j] = prod[ppos + j] - (long) sign_list[i] * pk[pkPos + j + PARAM_N - pos]; } for (j = pos; j < PARAM_N; j++) { - prod[ppos + j] = prod[ppos + j] + sign_list[i] * pk[pkPos + j - pos]; + prod[ppos + j] = prod[ppos + j] + (long) sign_list[i] * pk[pkPos + j - pos]; } } } diff --git a/core/src/main/java/org/bouncycastle/pqc/crypto/sphincs/Horst.java b/core/src/main/java/org/bouncycastle/pqc/crypto/sphincs/Horst.java index a24341d3cd..99294047a3 100644 --- a/core/src/main/java/org/bouncycastle/pqc/crypto/sphincs/Horst.java +++ b/core/src/main/java/org/bouncycastle/pqc/crypto/sphincs/Horst.java @@ -45,7 +45,7 @@ static int horst_sign(HashFunctions hs, offset_out = (1 << (HORST_LOGT - i - 1)) - 1; for (j = 0; j < (1 << (HORST_LOGT - i - 1)); j++) { - hs.hash_2n_n_mask(tree, (int)((offset_out + j) * SPHINCS256Config.HASH_BYTES), tree, (int)((offset_in + 2 * j) * SPHINCS256Config.HASH_BYTES), masks, 2 * i * SPHINCS256Config.HASH_BYTES); + hs.hash_2n_n_mask(tree, (int)((offset_out + j) * SPHINCS256Config.HASH_BYTES), tree, (int)((offset_in + 2L * j) * SPHINCS256Config.HASH_BYTES), masks, 2 * i * SPHINCS256Config.HASH_BYTES); } } diff --git a/core/src/main/java/org/bouncycastle/pqc/math/ntru/polynomial/IntegerPolynomial.java b/core/src/main/java/org/bouncycastle/pqc/math/ntru/polynomial/IntegerPolynomial.java index bcf8958cee..462004e681 100644 --- a/core/src/main/java/org/bouncycastle/pqc/math/ntru/polynomial/IntegerPolynomial.java +++ b/core/src/main/java/org/bouncycastle/pqc/math/ntru/polynomial/IntegerPolynomial.java @@ -859,7 +859,7 @@ private BigInteger squareSum() BigInteger sum = Constants.BIGINT_ZERO; for (int i = 0; i < coeffs.length; i++) { - sum = sum.add(BigInteger.valueOf(coeffs[i] * coeffs[i])); + sum = sum.add(BigInteger.valueOf((long) coeffs[i] * (long) coeffs[i])); } return sum; } @@ -1100,7 +1100,7 @@ public long centeredNormSq(int q) { int c = p.coeffs[i]; sum += c; - sqSum += c * c; + sqSum += (long) c * (long) c; } long centeredNormSq = sqSum - sum * sum / N;