More -Wshorten-64-to-32 fixes.
I had a rewrite of the decrepit ciphers (CAST and Blowfish) to use
CRYPTO_{load,store}_u32_be and drop the old macros, but this is probably
not worth the effort to review. Instead, just fix the type in the macro.
Bug: 516
Change-Id: I1cdecc16f6108a6235f90cf9c2198bc797c6716e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54985
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/bio/bio_mem.c b/crypto/bio/bio_mem.c
index f40a9a7..b2585c5 100644
--- a/crypto/bio/bio_mem.c
+++ b/crypto/bio/bio_mem.c
@@ -66,7 +66,7 @@
#include "../internal.h"
-BIO *BIO_new_mem_buf(const void *buf, int len) {
+BIO *BIO_new_mem_buf(const void *buf, ossl_ssize_t len) {
BIO *ret;
BUF_MEM *b;
const size_t size = len < 0 ? strlen((char *)buf) : (size_t)len;
diff --git a/crypto/bytestring/cbs.c b/crypto/bytestring/cbs.c
index 4e7f379..ddf1f67 100644
--- a/crypto/bytestring/cbs.c
+++ b/crypto/bytestring/cbs.c
@@ -136,7 +136,7 @@
if (!cbs_get_u(cbs, &v, 3)) {
return 0;
}
- *out = v;
+ *out = (uint32_t)v;
return 1;
}
@@ -145,7 +145,7 @@
if (!cbs_get_u(cbs, &v, 4)) {
return 0;
}
- *out = v;
+ *out = (uint32_t)v;
return 1;
}
diff --git a/crypto/fipsmodule/bn/exponentiation.c b/crypto/fipsmodule/bn/exponentiation.c
index 9b609b3..e03785c 100644
--- a/crypto/fipsmodule/bn/exponentiation.c
+++ b/crypto/fipsmodule/bn/exponentiation.c
@@ -396,7 +396,7 @@
//
// (with draws in between). Very small exponents are often selected
// with low Hamming weight, so we use w = 1 for b <= 23.
-static int BN_window_bits_for_exponent_size(int b) {
+static int BN_window_bits_for_exponent_size(size_t b) {
if (b > 671) {
return 6;
}
@@ -721,12 +721,14 @@
void bn_mod_exp_mont_small(BN_ULONG *r, const BN_ULONG *a, size_t num,
const BN_ULONG *p, size_t num_p,
const BN_MONT_CTX *mont) {
- if (num != (size_t)mont->N.width || num > BN_SMALL_MAX_WORDS) {
+ if (num != (size_t)mont->N.width || num > BN_SMALL_MAX_WORDS ||
+ num_p > ((size_t)-1) / BN_BITS2) {
abort();
}
assert(BN_is_odd(&mont->N));
- // Count the number of bits in |p|. Note this function treats |p| as public.
+ // Count the number of bits in |p|, skipping leading zeros. Note this function
+ // treats |p| as public.
while (num_p != 0 && p[num_p - 1] == 0) {
num_p--;
}
@@ -734,7 +736,7 @@
bn_from_montgomery_small(r, num, mont->RR.d, num, mont);
return;
}
- unsigned bits = BN_num_bits_word(p[num_p - 1]) + (num_p - 1) * BN_BITS2;
+ size_t bits = BN_num_bits_word(p[num_p - 1]) + (num_p - 1) * BN_BITS2;
assert(bits != 0);
// We exponentiate by looking at sliding windows of the exponent and
@@ -758,7 +760,7 @@
// |p| is non-zero, so at least one window is non-zero. To save some
// multiplications, defer initializing |r| until then.
int r_is_one = 1;
- unsigned wstart = bits - 1; // The top bit of the window.
+ size_t wstart = bits - 1; // The top bit of the window.
for (;;) {
if (!bn_is_bit_set_words(p, num_p, wstart)) {
if (!r_is_one) {
diff --git a/crypto/fipsmodule/bn/internal.h b/crypto/fipsmodule/bn/internal.h
index f7c181e..ec87dd8 100644
--- a/crypto/fipsmodule/bn/internal.h
+++ b/crypto/fipsmodule/bn/internal.h
@@ -690,9 +690,10 @@
// bn_mod_exp_mont_small sets |r| to |a|^|p| mod |mont->N|. It returns one on
// success and zero on programmer or internal error. Both inputs and outputs are
// in the Montgomery domain. |r| and |a| are |num| words long, which must be
-// |mont->N.width| and at most |BN_SMALL_MAX_WORDS|. |a| must be fully-reduced.
-// This function runs in time independent of |a|, but |p| and |mont->N| are
-// public values. |a| must be fully-reduced and may alias with |r|.
+// |mont->N.width| and at most |BN_SMALL_MAX_WORDS|. |num_p|, measured in bits,
+// must fit in |size_t|. |a| must be fully-reduced. This function runs in time
+// independent of |a|, but |p| and |mont->N| are public values. |a| must be
+// fully-reduced and may alias with |r|.
//
// Note this function differs from |BN_mod_exp_mont| which uses Montgomery
// reduction but takes input and output outside the Montgomery domain. Combine
diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c
index f7b37e9..de50567 100644
--- a/crypto/pkcs8/pkcs8_x509.c
+++ b/crypto/pkcs8/pkcs8_x509.c
@@ -782,7 +782,9 @@
}
for (;;) {
- int n = BIO_read(bio, &buf->data[used], buf->length - used);
+ size_t max_read = buf->length - used;
+ int n = BIO_read(bio, &buf->data[used],
+ max_read > INT_MAX ? INT_MAX : (int)max_read);
if (n < 0) {
if (used == 0) {
goto out;
diff --git a/crypto/poly1305/poly1305.c b/crypto/poly1305/poly1305.c
index 94853b8..12f49bb 100644
--- a/crypto/poly1305/poly1305.c
+++ b/crypto/poly1305/poly1305.c
@@ -241,7 +241,6 @@
void CRYPTO_poly1305_finish(poly1305_state *statep, uint8_t mac[16]) {
struct poly1305_state_st *state = poly1305_aligned_state(statep);
- uint64_t f0, f1, f2, f3;
uint32_t g0, g1, g2, g3, g4;
uint32_t b, nb;
@@ -294,22 +293,22 @@
state->h3 = (state->h3 & nb) | (g3 & b);
state->h4 = (state->h4 & nb) | (g4 & b);
- f0 = ((state->h0) | (state->h1 << 26)) +
- (uint64_t)CRYPTO_load_u32_le(&state->key[0]);
- f1 = ((state->h1 >> 6) | (state->h2 << 20)) +
- (uint64_t)CRYPTO_load_u32_le(&state->key[4]);
- f2 = ((state->h2 >> 12) | (state->h3 << 14)) +
- (uint64_t)CRYPTO_load_u32_le(&state->key[8]);
- f3 = ((state->h3 >> 18) | (state->h4 << 8)) +
- (uint64_t)CRYPTO_load_u32_le(&state->key[12]);
+ uint64_t f0 = ((state->h0) | (state->h1 << 26)) +
+ (uint64_t)CRYPTO_load_u32_le(&state->key[0]);
+ uint64_t f1 = ((state->h1 >> 6) | (state->h2 << 20)) +
+ (uint64_t)CRYPTO_load_u32_le(&state->key[4]);
+ uint64_t f2 = ((state->h2 >> 12) | (state->h3 << 14)) +
+ (uint64_t)CRYPTO_load_u32_le(&state->key[8]);
+ uint64_t f3 = ((state->h3 >> 18) | (state->h4 << 8)) +
+ (uint64_t)CRYPTO_load_u32_le(&state->key[12]);
- CRYPTO_store_u32_le(&mac[0], f0);
+ CRYPTO_store_u32_le(&mac[0], (uint32_t)f0);
f1 += (f0 >> 32);
- CRYPTO_store_u32_le(&mac[4], f1);
+ CRYPTO_store_u32_le(&mac[4], (uint32_t)f1);
f2 += (f1 >> 32);
- CRYPTO_store_u32_le(&mac[8], f2);
+ CRYPTO_store_u32_le(&mac[8], (uint32_t)f2);
f3 += (f2 >> 32);
- CRYPTO_store_u32_le(&mac[12], f3);
+ CRYPTO_store_u32_le(&mac[12], (uint32_t)f3);
}
#endif // !BORINGSSL_HAS_UINT128 || !OPENSSL_X86_64
diff --git a/decrepit/macros.h b/decrepit/macros.h
index 7888f0d..4e4ea93 100644
--- a/decrepit/macros.h
+++ b/decrepit/macros.h
@@ -61,35 +61,35 @@
// NOTE - c is not incremented as per n2l
-#define n2ln(c, l1, l2, n) \
- { \
- c += n; \
- l1 = l2 = 0; \
- switch (n) { \
- case 8: \
- l2 = ((unsigned long)(*(--(c)))); \
- OPENSSL_FALLTHROUGH; \
- case 7: \
- l2 |= ((unsigned long)(*(--(c)))) << 8; \
- OPENSSL_FALLTHROUGH; \
- case 6: \
- l2 |= ((unsigned long)(*(--(c)))) << 16; \
- OPENSSL_FALLTHROUGH; \
- case 5: \
- l2 |= ((unsigned long)(*(--(c)))) << 24; \
- OPENSSL_FALLTHROUGH; \
- case 4: \
- l1 = ((unsigned long)(*(--(c)))); \
- OPENSSL_FALLTHROUGH; \
- case 3: \
- l1 |= ((unsigned long)(*(--(c)))) << 8; \
- OPENSSL_FALLTHROUGH; \
- case 2: \
- l1 |= ((unsigned long)(*(--(c)))) << 16; \
- OPENSSL_FALLTHROUGH; \
- case 1: \
- l1 |= ((unsigned long)(*(--(c)))) << 24; \
- } \
+#define n2ln(c, l1, l2, n) \
+ { \
+ c += n; \
+ l1 = l2 = 0; \
+ switch (n) { \
+ case 8: \
+ l2 = ((uint32_t)(*(--(c)))); \
+ OPENSSL_FALLTHROUGH; \
+ case 7: \
+ l2 |= ((uint32_t)(*(--(c)))) << 8; \
+ OPENSSL_FALLTHROUGH; \
+ case 6: \
+ l2 |= ((uint32_t)(*(--(c)))) << 16; \
+ OPENSSL_FALLTHROUGH; \
+ case 5: \
+ l2 |= ((uint32_t)(*(--(c)))) << 24; \
+ OPENSSL_FALLTHROUGH; \
+ case 4: \
+ l1 = ((uint32_t)(*(--(c)))); \
+ OPENSSL_FALLTHROUGH; \
+ case 3: \
+ l1 |= ((uint32_t)(*(--(c)))) << 8; \
+ OPENSSL_FALLTHROUGH; \
+ case 2: \
+ l1 |= ((uint32_t)(*(--(c)))) << 16; \
+ OPENSSL_FALLTHROUGH; \
+ case 1: \
+ l1 |= ((uint32_t)(*(--(c)))) << 24; \
+ } \
}
// NOTE - c is not incremented as per l2n
@@ -99,25 +99,25 @@
switch (n) { \
case 8: \
*(--(c)) = (unsigned char)(((l2)) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 7: \
*(--(c)) = (unsigned char)(((l2) >> 8) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 6: \
*(--(c)) = (unsigned char)(((l2) >> 16) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 5: \
*(--(c)) = (unsigned char)(((l2) >> 24) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 4: \
*(--(c)) = (unsigned char)(((l1)) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 3: \
*(--(c)) = (unsigned char)(((l1) >> 8) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 2: \
*(--(c)) = (unsigned char)(((l1) >> 16) & 0xff); \
- OPENSSL_FALLTHROUGH; \
+ OPENSSL_FALLTHROUGH; \
case 1: \
*(--(c)) = (unsigned char)(((l1) >> 24) & 0xff); \
} \
@@ -129,11 +129,9 @@
*((c)++) = (unsigned char)(((l) >> 8L) & 0xff), \
*((c)++) = (unsigned char)(((l)) & 0xff))
-#define n2l(c, l) \
- (l = ((unsigned long)(*((c)++))) << 24L, \
- l |= ((unsigned long)(*((c)++))) << 16L, \
- l |= ((unsigned long)(*((c)++))) << 8L, \
- l |= ((unsigned long)(*((c)++))))
+#define n2l(c, l) \
+ (l = ((uint32_t)(*((c)++))) << 24L, l |= ((uint32_t)(*((c)++))) << 16L, \
+ l |= ((uint32_t)(*((c)++))) << 8L, l |= ((uint32_t)(*((c)++))))
#endif // OPENSSL_HEADER_DECREPIT_MACROS_H
diff --git a/include/openssl/bio.h b/include/openssl/bio.h
index 3de3e28..01ea69c 100644
--- a/include/openssl/bio.h
+++ b/include/openssl/bio.h
@@ -383,7 +383,7 @@
//
// If |len| is negative, then |buf| is treated as a NUL-terminated string, but
// don't depend on this in new code.
-OPENSSL_EXPORT BIO *BIO_new_mem_buf(const void *buf, int len);
+OPENSSL_EXPORT BIO *BIO_new_mem_buf(const void *buf, ossl_ssize_t len);
// BIO_mem_contents sets |*out_contents| to point to the current contents of
// |bio| and |*out_len| to contain the length of that data. It returns one on
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 72a28d9..6c8eba0 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2832,7 +2832,7 @@
// WARNING: this function is dangerous because it breaks the usual return value
// convention.
OPENSSL_EXPORT int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const uint8_t *protos,
- unsigned protos_len);
+ size_t protos_len);
// SSL_set_alpn_protos sets the client ALPN protocol list on |ssl| to |protos|.
// |protos| must be in wire-format (i.e. a series of non-empty, 8-bit
@@ -2843,7 +2843,7 @@
// WARNING: this function is dangerous because it breaks the usual return value
// convention.
OPENSSL_EXPORT int SSL_set_alpn_protos(SSL *ssl, const uint8_t *protos,
- unsigned protos_len);
+ size_t protos_len);
// SSL_CTX_set_alpn_select_cb sets a callback function on |ctx| that is called
// during ClientHello processing in order to select an ALPN protocol from the
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc
index 3424f6e..9a1567d 100644
--- a/ssl/d1_both.cc
+++ b/ssl/d1_both.cc
@@ -682,6 +682,7 @@
// Assemble a fragment, to be sealed in-place.
ScopedCBB cbb;
+ CBB child;
uint8_t *frag = out + prefix;
size_t max_frag = max_out - prefix, frag_len;
if (!CBB_init_fixed(cbb.get(), frag, max_frag) ||
@@ -689,8 +690,8 @@
!CBB_add_u24(cbb.get(), hdr.msg_len) ||
!CBB_add_u16(cbb.get(), hdr.seq) ||
!CBB_add_u24(cbb.get(), ssl->d1->outgoing_offset) ||
- !CBB_add_u24(cbb.get(), todo) ||
- !CBB_add_bytes(cbb.get(), CBS_data(&body), todo) ||
+ !CBB_add_u24_length_prefixed(cbb.get(), &child) ||
+ !CBB_add_bytes(&child, CBS_data(&body), todo) ||
!CBB_finish(cbb.get(), NULL, &frag_len)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return seal_error;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 4d56d37..cfd1862 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -143,6 +143,7 @@
#include <algorithm>
#include <assert.h>
+#include <limits.h>
#include <stdlib.h>
#include <string.h>
@@ -2199,8 +2200,10 @@
void SSL_get0_next_proto_negotiated(const SSL *ssl, const uint8_t **out_data,
unsigned *out_len) {
+ // NPN protocols have one-byte lengths, so they must fit in |unsigned|.
+ assert(ssl->s3->next_proto_negotiated.size() <= UINT_MAX);
*out_data = ssl->s3->next_proto_negotiated.data();
- *out_len = ssl->s3->next_proto_negotiated.size();
+ *out_len = static_cast<unsigned>(ssl->s3->next_proto_negotiated.size());
}
void SSL_CTX_set_next_protos_advertised_cb(
@@ -2220,7 +2223,7 @@
}
int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const uint8_t *protos,
- unsigned protos_len) {
+ size_t protos_len) {
// Note this function's return value is backwards.
auto span = MakeConstSpan(protos, protos_len);
if (!span.empty() && !ssl_is_valid_alpn_list(span)) {
@@ -2230,7 +2233,7 @@
return ctx->alpn_client_proto_list.CopyFrom(span) ? 0 : 1;
}
-int SSL_set_alpn_protos(SSL *ssl, const uint8_t *protos, unsigned protos_len) {
+int SSL_set_alpn_protos(SSL *ssl, const uint8_t *protos, size_t protos_len) {
// Note this function's return value is backwards.
if (!ssl->config) {
return 1;
@@ -2254,13 +2257,16 @@
void SSL_get0_alpn_selected(const SSL *ssl, const uint8_t **out_data,
unsigned *out_len) {
+ Span<const uint8_t> protocol;
if (SSL_in_early_data(ssl) && !ssl->server) {
- *out_data = ssl->s3->hs->early_session->early_alpn.data();
- *out_len = ssl->s3->hs->early_session->early_alpn.size();
+ protocol = ssl->s3->hs->early_session->early_alpn;
} else {
- *out_data = ssl->s3->alpn_selected.data();
- *out_len = ssl->s3->alpn_selected.size();
+ protocol = ssl->s3->alpn_selected;
}
+ // ALPN protocols have one-byte lengths, so they must fit in |unsigned|.
+ assert(protocol.size() < UINT_MAX);
+ *out_data = protocol.data();
+ *out_len = static_cast<unsigned>(protocol.size());
}
void SSL_CTX_set_allow_unknown_alpn_protos(SSL_CTX *ctx, int enabled) {
diff --git a/tool/transport_common.cc b/tool/transport_common.cc
index 991c808..e889688 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -843,7 +843,7 @@
}
if (i == 0) {
- *out_code = code;
+ *out_code = static_cast<unsigned>(code);
} else if (code != *out_code) {
fprintf(stderr,
"Reply code varied within a single reply: was %u, now %u\n",
