kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Avoid potential uninitialized memory read in crypto/ec/p256-x86_64.c.

Browse Source 
If the function returns early due to an error, then the coordinates of the
result will have their |top| value set to a value beyond what has actually
been been written. Fix that, and make it easier to avoid such issues in the
future by refactoring the code.

As a bonus, avoid a false positive MSVC 64-bit opt build "potentially
uninitialized value used" warning.

Change-Id: I8c48deb63163a27f739c8797962414f8ca2588cd
Reviewed-on: https://boringssl-review.googlesource.com/6579
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
Brian Smith 2016-03-08 17:09:40 -10:00 committed by David Benjamin
parent 081e3f34a2
commit d279a21d8c
3 changed files with 21 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
crypto/bn/bn.c
View File

@ -266,6 +266,17 @@ int BN_set_word(BIGNUM *bn, BN_ULONG value) {
return 1;
}
int bn_set_words(BIGNUM *bn, const BN_ULONG *words, size_t num) {
if (bn_wexpand(bn, num) == NULL) {
return 0;
}
memmove(bn->d, words, num * sizeof(BN_ULONG));
/* |bn_wexpand| verified that |num| isn't too large. */
bn->top = (int)num;
bn_correct_top(bn);
return 1;
}
int BN_is_negative(const BIGNUM *bn) {
return bn->neg != 0;
}

5
crypto/bn/internal.h
View File

@ -192,6 +192,11 @@ BIGNUM *bn_expand(BIGNUM *bn, size_t bits);
#define Hw(t) (((BN_ULONG)((t)>>BN_BITS2))&BN_MASK2)
#endif
/* bn_set_words sets |bn| to the value encoded in the |num| words in |words|,
* least significant word first. */
int bn_set_words(BIGNUM *bn, const BN_ULONG *words, size_t num);
BN_ULONG bn_mul_add_words(BN_ULONG *rp, const BN_ULONG *ap, int num, BN_ULONG w);
BN_ULONG bn_mul_words(BN_ULONG *rp, const BN_ULONG *ap, int num, BN_ULONG w);
void bn_sqr_words(BN_ULONG *rp, const BN_ULONG *ap, int num);

23
crypto/ec/p256-x86_64.c
View File

@ -390,17 +390,6 @@ static int ecp_nistz256_points_mul(
BN_CTX *new_ctx = NULL;
int ctx_started = 0;
/* Need 256 bits for space for all coordinates. */
if (bn_wexpand(&r->X, P256_LIMBS) == NULL ||
bn_wexpand(&r->Y, P256_LIMBS) == NULL ||
bn_wexpand(&r->Z, P256_LIMBS) == NULL) {
OPENSSL_PUT_ERROR(EC, ERR_R_MALLOC_FAILURE);
goto err;
}
r->X.top = P256_LIMBS;
r->Y.top = P256_LIMBS;
r->Z.top = P256_LIMBS;
if (g_scalar != NULL) {
if (BN_num_bits(g_scalar) > 256 || BN_is_negative(g_scalar)) {
if (ctx == NULL) {
@ -494,14 +483,12 @@ static int ecp_nistz256_points_mul(
}
}
memcpy(r->X.d, p.p.X, sizeof(p.p.X));
memcpy(r->Y.d, p.p.Y, sizeof(p.p.Y));
memcpy(r->Z.d, p.p.Z, sizeof(p.p.Z));
/* Not constant-time, but we're only operating on the public output. */
bn_correct_top(&r->X);
bn_correct_top(&r->Y);
bn_correct_top(&r->Z);
if (!bn_set_words(&r->X, p.p.X, P256_LIMBS) ||
!bn_set_words(&r->Y, p.p.Y, P256_LIMBS) ||
!bn_set_words(&r->Z, p.p.Z, P256_LIMBS)) {
return 0;
}
ret = 1;