Skip to content

Commit

Permalink
Rewrite bn_big_endian_to_words to avoid a GCC false positive
Browse files Browse the repository at this point in the history
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>
(cherry picked from commit 15a76eb224ec4eff94d00565ee7d13b1f5a3a6cc)
  • Loading branch information
davidben authored and skmcgrail committed Aug 23, 2024
1 parent 051da71 commit c31d1e2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 17 deletions.
37 changes: 21 additions & 16 deletions crypto/fipsmodule/bn/bytes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion crypto/fipsmodule/bn/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ void bn_mod_inverse0_prime_mont_small(BN_ULONG *r, const BN_ULONG *a,
// unsigned integer and writes the result to |out_len| words in |out|. The output
// is in little-endian word order with |out[0]| being the least-significant word.
// |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|.
// |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);

Expand Down

0 comments on commit c31d1e2

Please sign in to comment.