Rewrite bn_big_endian_to_words to avoid a GCC false positive
I got a -Wstringop-overflow warning in GCC 12.2.0, targetting 32-bit
Arm. It's a false positive, but rewriting the function this way seems a
bit clearer. (Previously, I tried to write it in a way that truncated if
the bounds were wrong. Just make it a BSSL_CHECK.)
Change-Id: Iaa3955f08f320f2c376ca66703e4dd29481128fd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65867
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/bn/bytes.c b/crypto/fipsmodule/bn/bytes.c
index aca0e38..d7f70da 100644
--- a/crypto/fipsmodule/bn/bytes.c
+++ b/crypto/fipsmodule/bn/bytes.c
@@ -63,26 +63,31 @@
void bn_big_endian_to_words(BN_ULONG *out, size_t out_len, const uint8_t *in,
size_t in_len) {
- for (size_t i = 0; i < out_len; i++) {
- if (in_len < sizeof(BN_ULONG)) {
- // Load the last partial word.
- BN_ULONG word = 0;
- for (size_t j = 0; j < in_len; j++) {
- word = (word << 8) | in[j];
- }
- in_len = 0;
- out[i] = word;
- // Fill the remainder with zeros.
- OPENSSL_memset(out + i + 1, 0, (out_len - i - 1) * sizeof(BN_ULONG));
- break;
- }
+ // The caller should have sized |out| to fit |in| without truncating. This
+ // condition ensures we do not overflow |out|, so use a runtime check.
+ BSSL_CHECK(in_len <= out_len * sizeof(BN_ULONG));
+ // Load whole words.
+ while (in_len >= sizeof(BN_ULONG)) {
in_len -= sizeof(BN_ULONG);
- out[i] = CRYPTO_load_word_be(in + in_len);
+ out[0] = CRYPTO_load_word_be(in + in_len);
+ out++;
+ out_len--;
}
- // The caller should have sized the output to avoid truncation.
- assert(in_len == 0);
+ // Load the last partial word.
+ if (in_len != 0) {
+ BN_ULONG word = 0;
+ for (size_t i = 0; i < in_len; i++) {
+ word = (word << 8) | in[i];
+ }
+ out[0] = word;
+ out++;
+ out_len--;
+ }
+
+ // Fill the remainder with zeros.
+ OPENSSL_memset(out, 0, out_len * sizeof(BN_ULONG));
}
BIGNUM *BN_bin2bn(const uint8_t *in, size_t len, BIGNUM *ret) {
diff --git a/crypto/fipsmodule/bn/internal.h b/crypto/fipsmodule/bn/internal.h
index a0133ef..363a97e 100644
--- a/crypto/fipsmodule/bn/internal.h
+++ b/crypto/fipsmodule/bn/internal.h
@@ -793,8 +793,8 @@
// bn_big_endian_to_words interprets |in_len| bytes from |in| as a big-endian,
// unsigned integer and writes the result to |out_len| words in |out|. |out_len|
-// must be large enough to represent any |in_len|-byte value. That is, |out_len|
-// must be at least |BN_BYTES * in_len|.
+// must be large enough to represent any |in_len|-byte value. That is, |in_len|
+// must be at most |BN_BYTES * out_len|.
void bn_big_endian_to_words(BN_ULONG *out, size_t out_len, const uint8_t *in,
size_t in_len);
