Various -Wshorten-64-to-32 fixes.
This is far from all of it, but finishes a good chunk of bcm.c.
Bug: 516
Change-Id: If764e5af1c6b62e8342554502ecc4d563e44bc50
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54207
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/aes/aes_nohw.c b/crypto/fipsmodule/aes/aes_nohw.c
index b5990b8..c5dec0e 100644
--- a/crypto/fipsmodule/aes/aes_nohw.c
+++ b/crypto/fipsmodule/aes/aes_nohw.c
@@ -1192,7 +1192,7 @@
for (;;) {
// Update counters.
for (size_t i = 0; i < AES_NOHW_BATCH_SIZE; i++) {
- CRYPTO_store_u32_be(ivs + 16 * i + 12, ctr + i);
+ CRYPTO_store_u32_be(ivs + 16 * i + 12, ctr + (uint32_t)i);
}
size_t todo = blocks >= AES_NOHW_BATCH_SIZE ? AES_NOHW_BATCH_SIZE : blocks;
diff --git a/crypto/fipsmodule/bn/bn.c b/crypto/fipsmodule/bn/bn.c
index f2e3b7b..f3fbb7a 100644
--- a/crypto/fipsmodule/bn/bn.c
+++ b/crypto/fipsmodule/bn/bn.c
@@ -67,6 +67,11 @@
#include "../delocate.h"
+// BN_MAX_WORDS is the maximum number of words allowed in a |BIGNUM|. It is
+// sized so byte and bit counts of a |BIGNUM| always fit in |int|, with room to
+// spare.
+#define BN_MAX_WORDS (INT_MAX / (4 * BN_BITS2))
+
BIGNUM *BN_new(void) {
BIGNUM *bn = OPENSSL_malloc(sizeof(BIGNUM));
@@ -292,8 +297,9 @@
}
bn->d = (BN_ULONG *)words;
- bn->width = num;
- bn->dmax = num;
+ assert(num <= BN_MAX_WORDS);
+ bn->width = (int)num;
+ bn->dmax = (int)num;
bn->neg = 0;
bn->flags |= BN_FLG_STATIC_DATA;
}
@@ -346,7 +352,7 @@
return 1;
}
- if (words > (INT_MAX / (4 * BN_BITS2))) {
+ if (words > BN_MAX_WORDS) {
OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
return 0;
}
@@ -403,7 +409,7 @@
}
OPENSSL_memset(bn->d + bn->width, 0,
(words - bn->width) * sizeof(BN_ULONG));
- bn->width = words;
+ bn->width = (int)words;
return 1;
}
@@ -412,7 +418,7 @@
OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
return 0;
}
- bn->width = words;
+ bn->width = (int)words;
return 1;
}
diff --git a/crypto/fipsmodule/bn/bytes.c b/crypto/fipsmodule/bn/bytes.c
index 38d71a3..da27426 100644
--- a/crypto/fipsmodule/bn/bytes.c
+++ b/crypto/fipsmodule/bn/bytes.c
@@ -138,7 +138,7 @@
BN_free(bn);
return NULL;
}
- ret->width = num_words;
+ ret->width = (int)num_words;
// Make sure the top bytes will be zeroed.
ret->d[num_words - 1] = 0;
diff --git a/crypto/fipsmodule/bn/div.c b/crypto/fipsmodule/bn/div.c
index 02b9931..6de5a43 100644
--- a/crypto/fipsmodule/bn/div.c
+++ b/crypto/fipsmodule/bn/div.c
@@ -552,7 +552,7 @@
return NULL;
}
ret->neg = 0;
- ret->width = width;
+ ret->width = (int)width;
return ret;
}
diff --git a/crypto/fipsmodule/bn/div_extra.c b/crypto/fipsmodule/bn/div_extra.c
index 7f03f28..141f7da 100644
--- a/crypto/fipsmodule/bn/div_extra.c
+++ b/crypto/fipsmodule/bn/div_extra.c
@@ -70,7 +70,7 @@
// This operation is not constant-time, but |p| and |d| are public values.
// Note that |p| is at most 16, so the computation fits in |uint64_t|.
assert(p <= 16);
- uint32_t m = ((UINT64_C(1) << (32 + p)) + d - 1) / d;
+ uint32_t m = (uint32_t)(((UINT64_C(1) << (32 + p)) + d - 1) / d);
uint16_t ret = 0;
for (int i = bn->width - 1; i >= 0; i--) {
diff --git a/crypto/fipsmodule/bn/gcd_extra.c b/crypto/fipsmodule/bn/gcd_extra.c
index 53ab170..b4fe00e 100644
--- a/crypto/fipsmodule/bn/gcd_extra.c
+++ b/crypto/fipsmodule/bn/gcd_extra.c
@@ -240,8 +240,10 @@
// Each loop iteration halves at least one of |u| and |v|. Thus we need at
// most the combined bit width of inputs for at least one value to be zero.
- unsigned a_bits = a_width * BN_BITS2, n_bits = n_width * BN_BITS2;
- unsigned num_iters = a_bits + n_bits;
+ // |a_bits| and |n_bits| cannot overflow because |bn_wexpand| ensures bit
+ // counts fit in even |int|.
+ size_t a_bits = a_width * BN_BITS2, n_bits = n_width * BN_BITS2;
+ size_t num_iters = a_bits + n_bits;
if (num_iters < a_bits) {
OPENSSL_PUT_ERROR(BN, BN_R_BIGNUM_TOO_LONG);
goto err;
@@ -260,7 +262,7 @@
//
// After each loop iteration, u and v only get smaller, and at least one of
// them shrinks by at least a factor of two.
- for (unsigned i = 0; i < num_iters; i++) {
+ for (size_t i = 0; i < num_iters; i++) {
BN_ULONG both_odd = word_is_odd_mask(u->d[0]) & word_is_odd_mask(v->d[0]);
// If both |u| and |v| are odd, subtract the smaller from the larger.
diff --git a/crypto/fipsmodule/bn/internal.h b/crypto/fipsmodule/bn/internal.h
index 50fd362..f7c181e 100644
--- a/crypto/fipsmodule/bn/internal.h
+++ b/crypto/fipsmodule/bn/internal.h
@@ -439,7 +439,7 @@
// bn_is_bit_set_words returns one if bit |bit| is set in |a| and zero
// otherwise.
-int bn_is_bit_set_words(const BN_ULONG *a, size_t num, unsigned bit);
+int bn_is_bit_set_words(const BN_ULONG *a, size_t num, size_t bit);
// bn_one_to_montgomery sets |r| to one in Montgomery form. It returns one on
// success and zero on error. This function treats the bit width of the modulus
diff --git a/crypto/fipsmodule/bn/random.c b/crypto/fipsmodule/bn/random.c
index 4966778..2500cc1 100644
--- a/crypto/fipsmodule/bn/random.c
+++ b/crypto/fipsmodule/bn/random.c
@@ -336,7 +336,7 @@
assert(bn_in_range_words(r->d, min_inclusive, max_exclusive->d, words));
r->neg = 0;
- r->width = words;
+ r->width = (int)words;
return 1;
}
diff --git a/crypto/fipsmodule/bn/shift.c b/crypto/fipsmodule/bn/shift.c
index 55f864e..6960e57 100644
--- a/crypto/fipsmodule/bn/shift.c
+++ b/crypto/fipsmodule/bn/shift.c
@@ -258,9 +258,9 @@
return 1;
}
-int bn_is_bit_set_words(const BN_ULONG *a, size_t num, unsigned bit) {
- unsigned i = bit / BN_BITS2;
- unsigned j = bit % BN_BITS2;
+int bn_is_bit_set_words(const BN_ULONG *a, size_t num, size_t bit) {
+ size_t i = bit / BN_BITS2;
+ size_t j = bit % BN_BITS2;
if (i >= num) {
return 0;
}
diff --git a/crypto/fipsmodule/cipher/e_aes.c b/crypto/fipsmodule/cipher/e_aes.c
index 8d5ed4c..5f23793 100644
--- a/crypto/fipsmodule/cipher/e_aes.c
+++ b/crypto/fipsmodule/cipher/e_aes.c
@@ -47,6 +47,7 @@
* ==================================================================== */
#include <assert.h>
+#include <limits.h>
#include <string.h>
#include <openssl/aead.h>
@@ -289,8 +290,10 @@
ctr128_f aes_ctr_set_key(AES_KEY *aes_key, GCM128_KEY *gcm_key,
block128_f *out_block, const uint8_t *key,
size_t key_bytes) {
+ // This function assumes the key length was previously validated.
+ assert(key_bytes == 128 / 8 || key_bytes == 192 / 8 || key_bytes == 256 / 8);
if (hwaes_capable()) {
- aes_hw_set_encrypt_key(key, key_bytes * 8, aes_key);
+ aes_hw_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_hw_encrypt, 1);
}
@@ -301,7 +304,7 @@
}
if (vpaes_capable()) {
- vpaes_set_encrypt_key(key, key_bytes * 8, aes_key);
+ vpaes_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
if (out_block) {
*out_block = vpaes_encrypt;
}
@@ -318,7 +321,7 @@
#endif
}
- aes_nohw_set_encrypt_key(key, key_bytes * 8, aes_key);
+ aes_nohw_set_encrypt_key(key, (int)key_bytes * 8, aes_key);
if (gcm_key != NULL) {
CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_nohw_encrypt, 0);
}
@@ -552,6 +555,14 @@
return -1;
}
+ if (len > INT_MAX) {
+ // This function signature can only express up to |INT_MAX| bytes encrypted.
+ //
+ // TODO(https://crbug.com/boringssl/494): Make the internal |EVP_CIPHER|
+ // calling convention |size_t|-clean.
+ return -1;
+ }
+
if (in) {
if (out == NULL) {
if (!CRYPTO_gcm128_aad(&gctx->gcm, in, len)) {
@@ -580,7 +591,7 @@
}
}
}
- return len;
+ return (int)len;
} else {
if (!ctx->encrypt) {
if (gctx->taglen < 0 ||
diff --git a/crypto/fipsmodule/cmac/cmac.c b/crypto/fipsmodule/cmac/cmac.c
index f5c805d..efffe5f 100644
--- a/crypto/fipsmodule/cmac/cmac.c
+++ b/crypto/fipsmodule/cmac/cmac.c
@@ -49,6 +49,7 @@
#include <openssl/cmac.h>
#include <assert.h>
+#include <limits.h>
#include <string.h>
#include <openssl/aes.h>
@@ -269,7 +270,10 @@
}
OPENSSL_memcpy(ctx->block, in, in_len);
- ctx->block_used = in_len;
+ // |in_len| is bounded by |block_size|, which fits in |unsigned|.
+ static_assert(EVP_MAX_BLOCK_LENGTH < UINT_MAX,
+ "EVP_MAX_BLOCK_LENGTH is too large");
+ ctx->block_used = (unsigned)in_len;
ret = 1;
out:
diff --git a/crypto/fipsmodule/dh/dh.c b/crypto/fipsmodule/dh/dh.c
index 31eaff4..1a3c974 100644
--- a/crypto/fipsmodule/dh/dh.c
+++ b/crypto/fipsmodule/dh/dh.c
@@ -368,7 +368,8 @@
int ret = -1;
BIGNUM *shared_key = BN_CTX_get(ctx);
if (shared_key && dh_compute_key(dh, shared_key, peers_key, ctx)) {
- ret = BN_bn2bin(shared_key, out);
+ // A |BIGNUM|'s byte count fits in |int|.
+ ret = (int)BN_bn2bin(shared_key, out);
}
BN_CTX_end(ctx);
diff --git a/crypto/fipsmodule/rand/ctrdrbg.c b/crypto/fipsmodule/rand/ctrdrbg.c
index 0e8995f..805372f 100644
--- a/crypto/fipsmodule/rand/ctrdrbg.c
+++ b/crypto/fipsmodule/rand/ctrdrbg.c
@@ -184,7 +184,7 @@
OPENSSL_memset(out, 0, todo);
ctr32_add(drbg, 1);
drbg->ctr(out, out, num_blocks, &drbg->ks, drbg->counter);
- ctr32_add(drbg, num_blocks - 1);
+ ctr32_add(drbg, (uint32_t)(num_blocks - 1));
} else {
for (size_t i = 0; i < todo; i += AES_BLOCK_SIZE) {
ctr32_add(drbg, 1);
diff --git a/crypto/fipsmodule/rsa/rsa.c b/crypto/fipsmodule/rsa/rsa.c
index 733e7fa..14cdae5 100644
--- a/crypto/fipsmodule/rsa/rsa.c
+++ b/crypto/fipsmodule/rsa/rsa.c
@@ -300,7 +300,7 @@
OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
return -1;
}
- return out_len;
+ return (int)out_len;
}
static int rsa_sign_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
@@ -332,7 +332,7 @@
OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
return -1;
}
- return out_len;
+ return (int)out_len;
}
int RSA_decrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
@@ -347,7 +347,6 @@
int RSA_private_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
int padding) {
size_t out_len;
-
if (!RSA_decrypt(rsa, &out_len, to, RSA_size(rsa), from, flen, padding)) {
return -1;
}
@@ -356,13 +355,12 @@
OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
return -1;
}
- return out_len;
+ return (int)out_len;
}
int RSA_public_decrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
int padding) {
size_t out_len;
-
if (!RSA_verify_raw(rsa, &out_len, to, RSA_size(rsa), from, flen, padding)) {
return -1;
}
@@ -371,15 +369,16 @@
OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
return -1;
}
- return out_len;
+ return (int)out_len;
}
unsigned RSA_size(const RSA *rsa) {
- if (rsa->meth->size) {
- return rsa->meth->size(rsa);
- }
-
- return rsa_default_size(rsa);
+ size_t ret = rsa->meth->size ? rsa->meth->size(rsa) : rsa_default_size(rsa);
+ // RSA modulus sizes are bounded by |BIGNUM|, which must fit in |unsigned|.
+ //
+ // TODO(https://crbug.com/boringssl/516): Should we make this return |size_t|?
+ assert(ret < UINT_MAX);
+ return (unsigned)ret;
}
int RSA_is_opaque(const RSA *rsa) {
@@ -474,8 +473,6 @@
int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
int *is_alloced, int hash_nid, const uint8_t *digest,
size_t digest_len) {
- unsigned i;
-
if (hash_nid == NID_md5_sha1) {
// Special case: SSL signature, just check the length.
if (digest_len != SSL_SIG_LENGTH) {
@@ -489,7 +486,7 @@
return 1;
}
- for (i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
+ for (size_t i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
const struct pkcs1_sig_prefix *sig_prefix = &kPKCS1SigPrefixes[i];
if (sig_prefix->nid != hash_nid) {
continue;
@@ -501,17 +498,14 @@
}
const uint8_t* prefix = sig_prefix->bytes;
- unsigned prefix_len = sig_prefix->len;
- unsigned signed_msg_len;
- uint8_t *signed_msg;
-
- signed_msg_len = prefix_len + digest_len;
+ size_t prefix_len = sig_prefix->len;
+ size_t signed_msg_len = prefix_len + digest_len;
if (signed_msg_len < prefix_len) {
OPENSSL_PUT_ERROR(RSA, RSA_R_TOO_LONG);
return 0;
}
- signed_msg = OPENSSL_malloc(signed_msg_len);
+ uint8_t *signed_msg = OPENSSL_malloc(signed_msg_len);
if (!signed_msg) {
OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
return 0;
@@ -554,7 +548,12 @@
goto err;
}
- *out_len = size_t_out_len;
+ if (size_t_out_len > UINT_MAX) {
+ OPENSSL_PUT_ERROR(RSA, ERR_R_OVERFLOW);
+ goto err;
+ }
+
+ *out_len = (unsigned)size_t_out_len;
ret = 1;
err:
diff --git a/crypto/fipsmodule/tls/kdf.c b/crypto/fipsmodule/tls/kdf.c
index 6f2b68b..046cb52 100644
--- a/crypto/fipsmodule/tls/kdf.c
+++ b/crypto/fipsmodule/tls/kdf.c
@@ -91,7 +91,7 @@
}
for (;;) {
- unsigned len;
+ unsigned len_u;
uint8_t hmac[EVP_MAX_MD_SIZE];
if (!HMAC_CTX_copy_ex(&ctx, &ctx_init) ||
!HMAC_Update(&ctx, A1, A1_len) ||
@@ -100,16 +100,17 @@
!HMAC_Update(&ctx, (const uint8_t *) label, label_len) ||
!HMAC_Update(&ctx, seed1, seed1_len) ||
!HMAC_Update(&ctx, seed2, seed2_len) ||
- !HMAC_Final(&ctx, hmac, &len)) {
+ !HMAC_Final(&ctx, hmac, &len_u)) {
goto err;
}
+ size_t len = len_u;
assert(len == chunk);
// XOR the result into |out|.
if (len > out_len) {
len = out_len;
}
- for (unsigned i = 0; i < len; i++) {
+ for (size_t i = 0; i < len; i++) {
out[i] ^= hmac[i];
}
out += len;
diff --git a/crypto/test/file_test.cc b/crypto/test/file_test.cc
index 47a6f4c..e1ab2aa 100644
--- a/crypto/test/file_test.cc
+++ b/crypto/test/file_test.cc
@@ -20,6 +20,7 @@
#include <assert.h>
#include <ctype.h>
#include <errno.h>
+#include <limits.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
@@ -377,7 +378,8 @@
return FileTest::kReadError;
}
- if (fgets(out, len, file_) == nullptr) {
+ len = std::min(len, size_t{INT_MAX});
+ if (fgets(out, static_cast<int>(len), file_) == nullptr) {
return feof(file_) ? FileTest::kReadEOF : FileTest::kReadError;
}
diff --git a/crypto/test/wycheproof_util.cc b/crypto/test/wycheproof_util.cc
index cd870dd..9d31706 100644
--- a/crypto/test/wycheproof_util.cc
+++ b/crypto/test/wycheproof_util.cc
@@ -14,6 +14,7 @@
#include "./wycheproof_util.h"
+#include <limits.h>
#include <stdlib.h>
#include <algorithm>
@@ -138,7 +139,8 @@
return nullptr;
}
BIGNUM *bn = nullptr;
- if (BN_hex2bn(&bn, value.c_str()) != static_cast<int>(value.size())) {
+ if (value.size() > INT_MAX ||
+ BN_hex2bn(&bn, value.c_str()) != static_cast<int>(value.size())) {
BN_free(bn);
t->PrintLine("Could not decode value '%s'", value.c_str());
return nullptr;
@@ -151,8 +153,9 @@
// https://github.com/google/wycheproof/blob/0329f5b751ef102bd6b7b7181b6e049522a887f5/java/com/google/security/wycheproof/JsonUtil.java#L62.
if ('0' > value[0] || value[0] > '7') {
bssl::UniquePtr<BIGNUM> tmp(BN_new());
- if (!tmp ||
- !BN_set_bit(tmp.get(), value.size() * 4) ||
+ if (!tmp || //
+ value.size() > INT_MAX / 4 ||
+ !BN_set_bit(tmp.get(), static_cast<int>(value.size() * 4)) ||
!BN_sub(ret.get(), ret.get(), tmp.get())) {
return nullptr;
}
diff --git a/include/openssl/bn.h b/include/openssl/bn.h
index 3a721fe..0f381af 100644
--- a/include/openssl/bn.h
+++ b/include/openssl/bn.h
@@ -205,6 +205,10 @@
// BN_num_bytes returns the minimum number of bytes needed to represent the
// absolute value of |bn|.
+//
+// While |size_t| is the preferred type for byte counts, callers can assume that
+// |BIGNUM|s are bounded such that this value, and its corresponding bit count,
+// will always fit in |int|.
OPENSSL_EXPORT unsigned BN_num_bytes(const BIGNUM *bn);
// BN_zero sets |bn| to zero.
