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>

2 files changed, 23 insertions, 18 deletions

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--;
+  }
+
+  // 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--;
   }
 
-  // The caller should have sized the output to avoid truncation.
-  assert(in_len == 0);
+  // 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 @@ void bn_mod_inverse0_prime_mont_small(BN_ULONG *r, const BN_ULONG *a,
 
 // 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);