diff options
author | David Benjamin <davidben@google.com> | 2023-02-27 16:32:32 -0500 |
---|---|---|
committer | Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2023-02-27 21:49:02 +0000 |
commit | 3c6085b6ae982a80633bf5369c274036702c6848 (patch) | |
tree | 2e58a272d0b4a5ad7ae2046be8576c7ca3e22240 /crypto/asn1 | |
parent | 92859cc7484ac178e77825ccb86f95edcf0b6c49 (diff) | |
download | boringssl-3c6085b6ae982a80633bf5369c274036702c6848.zip boringssl-3c6085b6ae982a80633bf5369c274036702c6848.tar.gz boringssl-3c6085b6ae982a80633bf5369c274036702c6848.tar.bz2 |
Workaround yet more NULL + 0 language bugs
No new tests because they're actually caught by our own tests. I just
forgot to put UBSan on CI! Will fix this shortly.
For BLAKE2, fix this by checking for zero. For c2i_ASN1_INTEGER,
rewrite it with CBS, which has the side effect of avoiding this. (It's
effectively maintaining in->data + start as a temporary, rather than
start itself.)
Bug: fuchsia:46910
Change-Id: I9366f1ba4fd0b0140d64c56e0534d7b060ab90e5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57687
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Diffstat (limited to 'crypto/asn1')
-rw-r--r-- | crypto/asn1/a_int.c | 26 |
1 files changed, 14 insertions, 12 deletions
diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c index 78d1f09..27845c0 100644 --- a/crypto/asn1/a_int.c +++ b/crypto/asn1/a_int.c @@ -122,14 +122,17 @@ int i2c_ASN1_INTEGER(const ASN1_INTEGER *in, unsigned char **outp) { // |ASN1_INTEGER|s should be represented minimally, but it is possible to // construct invalid ones. Skip leading zeros so this does not produce an // invalid encoding or break invariants. - int start = 0; - while (start < in->length && in->data[start] == 0) { - start++; + CBS cbs; + CBS_init(&cbs, in->data, in->length); + while (CBS_len(&cbs) > 0 && CBS_data(&cbs)[0] == 0) { + CBS_skip(&cbs, 1); } int is_negative = (in->type & V_ASN1_NEG) != 0; - int pad; - if (start >= in->length) { + size_t pad; + CBS copy = cbs; + uint8_t msb; + if (!CBS_get_u8(©, &msb)) { // Zero is represented as a single byte. is_negative = 0; pad = 1; @@ -138,20 +141,19 @@ int i2c_ASN1_INTEGER(const ASN1_INTEGER *in, unsigned char **outp) { // through 0x00...01 and need an extra byte to be negative. // 0x01...00 through 0x80...00 have a two's complement of 0xfe...ff // through 0x80...00 and can be negated as-is. - pad = in->data[start] > 0x80 || - (in->data[start] == 0x80 && - !is_all_zeros(in->data + start + 1, in->length - start - 1)); + pad = msb > 0x80 || + (msb == 0x80 && !is_all_zeros(CBS_data(©), CBS_len(©))); } else { // If the high bit is set, the signed representation needs an extra // byte to be positive. - pad = (in->data[start] & 0x80) != 0; + pad = (msb & 0x80) != 0; } - if (in->length - start > INT_MAX - pad) { + if (CBS_len(&cbs) > INT_MAX - pad) { OPENSSL_PUT_ERROR(ASN1, ERR_R_OVERFLOW); return 0; } - int len = pad + in->length - start; + int len = (int)(pad + CBS_len(&cbs)); assert(len > 0); if (outp == NULL) { return len; @@ -160,7 +162,7 @@ int i2c_ASN1_INTEGER(const ASN1_INTEGER *in, unsigned char **outp) { if (pad) { (*outp)[0] = 0; } - OPENSSL_memcpy(*outp + pad, in->data + start, in->length - start); + OPENSSL_memcpy(*outp + pad, CBS_data(&cbs), CBS_len(&cbs)); if (is_negative) { negate_twos_complement(*outp, len); assert((*outp)[0] >= 0x80); |