diff --git a/crypto/fipsmodule/sha/sha256.c b/crypto/fipsmodule/sha/sha256.c index 454b947..046f6e2 100644 --- a/crypto/fipsmodule/sha/sha256.c +++ b/crypto/fipsmodule/sha/sha256.c
@@ -133,7 +133,7 @@ return SHA256_Update(ctx, data, len); } -static int sha256_final_impl(uint8_t *out, SHA256_CTX *c) { +static int sha256_final_impl(uint8_t *out, size_t md_len, SHA256_CTX *c) { crypto_md32_final(&sha256_block_data_order, c->h, c->data, SHA256_CBLOCK, &c->num, c->Nh, c->Nl, /*is_big_endian=*/1); @@ -141,12 +141,12 @@ // 'final' function can fail. SHA-512 does not have a corresponding check. // These functions already misbehave if the caller arbitrarily mutates |c|, so // can we assume one of |SHA256_Init| or |SHA224_Init| was used? - if (c->md_len > SHA256_DIGEST_LENGTH) { + if (md_len > SHA256_DIGEST_LENGTH) { return 0; } - assert(c->md_len % 4 == 0); - const size_t out_words = c->md_len / 4; + assert(md_len % 4 == 0); + const size_t out_words = md_len / 4; for (size_t i = 0; i < out_words; i++) { CRYPTO_store_u32_be(out, c->h[i]); out += 4; @@ -162,14 +162,14 @@ // |SHA256_Final| and expects |sha->md_len| to carry the size over. // // TODO(davidben): Add an assert and fix code to match them up. - return sha256_final_impl(out, c); + return sha256_final_impl(out, c->md_len, c); } int SHA224_Final(uint8_t out[SHA224_DIGEST_LENGTH], SHA256_CTX *ctx) { - // SHA224_Init sets |ctx->md_len| to |SHA224_DIGEST_LENGTH|, so this has a - // smaller output. + // This function must be paired with |SHA224_Init|, which sets |ctx->md_len| + // to |SHA224_DIGEST_LENGTH|. assert(ctx->md_len == SHA224_DIGEST_LENGTH); - return sha256_final_impl(out, ctx); + return sha256_final_impl(out, SHA224_DIGEST_LENGTH, ctx); } #ifndef SHA256_ASM
diff --git a/crypto/fipsmodule/sha/sha512.c b/crypto/fipsmodule/sha/sha512.c index 708358e..2c7ce31 100644 --- a/crypto/fipsmodule/sha/sha512.c +++ b/crypto/fipsmodule/sha/sha512.c
@@ -71,7 +71,7 @@ // this writing, so there is no need for a common collector/padding // implementation yet. -static int sha512_final_impl(uint8_t *out, SHA512_CTX *sha); +static int sha512_final_impl(uint8_t *out, size_t md_len, SHA512_CTX *sha); int SHA384_Init(SHA512_CTX *sha) { sha->h[0] = UINT64_C(0xcbbb9d5dc1059ed8); @@ -162,10 +162,10 @@ int SHA384_Final(uint8_t out[SHA384_DIGEST_LENGTH], SHA512_CTX *sha) { - // |SHA384_Init| sets |sha->md_len| to |SHA384_DIGEST_LENGTH|, so this has a - // smaller output. + // This function must be paired with |SHA384_Init|, which sets |sha->md_len| + // to |SHA384_DIGEST_LENGTH|. assert(sha->md_len == SHA384_DIGEST_LENGTH); - return sha512_final_impl(out, sha); + return sha512_final_impl(out, SHA384_DIGEST_LENGTH, sha); } int SHA384_Update(SHA512_CTX *sha, const void *data, size_t len) { @@ -177,10 +177,10 @@ } int SHA512_256_Final(uint8_t out[SHA512_256_DIGEST_LENGTH], SHA512_CTX *sha) { - // |SHA512_256_Init| sets |sha->md_len| to |SHA512_256_DIGEST_LENGTH|, so this - // has a |smaller output. + // This function must be paired with |SHA512_256_Init|, which sets + // |sha->md_len| to |SHA512_256_DIGEST_LENGTH|. assert(sha->md_len == SHA512_256_DIGEST_LENGTH); - return sha512_final_impl(out, sha); + return sha512_final_impl(out, SHA512_256_DIGEST_LENGTH, sha); } void SHA512_Transform(SHA512_CTX *c, const uint8_t block[SHA512_CBLOCK]) { @@ -241,10 +241,10 @@ // |SHA512_Final| and expects |sha->md_len| to carry the size over. // // TODO(davidben): Add an assert and fix code to match them up. - return sha512_final_impl(out, sha); + return sha512_final_impl(out, sha->md_len, sha); } -static int sha512_final_impl(uint8_t *out, SHA512_CTX *sha) { +static int sha512_final_impl(uint8_t *out, size_t md_len, SHA512_CTX *sha) { uint8_t *p = sha->p; size_t n = sha->num; @@ -268,8 +268,8 @@ return 0; } - assert(sha->md_len % 8 == 0); - const size_t out_words = sha->md_len / 8; + assert(md_len % 8 == 0); + const size_t out_words = md_len / 8; for (size_t i = 0; i < out_words; i++) { CRYPTO_store_u64_be(out, sha->h[i]); out += 8;
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c index 15ebe54..76df91c 100644 --- a/crypto/x509v3/v3_utl.c +++ b/crypto/x509v3/v3_utl.c
@@ -1233,8 +1233,6 @@ return 0; } - // Now for some sanity checks - if (v6stat.zero_pos == -1) { // If no '::' must have exactly 16 bytes if (v6stat.total != 16) { @@ -1242,35 +1240,31 @@ } } else { // If '::' must have less than 16 bytes - if (v6stat.total == 16) { + if (v6stat.total >= 16) { return 0; } - // More than three zeroes is an error if (v6stat.zero_cnt > 3) { + // More than three zeroes is an error return 0; - } - // Can only have three zeroes if nothing else present - else if (v6stat.zero_cnt == 3) { + } else if (v6stat.zero_cnt == 3) { + // Can only have three zeroes if nothing else present if (v6stat.total > 0) { return 0; } - } - // Can only have two zeroes if at start or end - else if (v6stat.zero_cnt == 2) { - if ((v6stat.zero_pos != 0) && (v6stat.zero_pos != v6stat.total)) { + } else if (v6stat.zero_cnt == 2) { + // Can only have two zeroes if at start or end + if (v6stat.zero_pos != 0 && v6stat.zero_pos != v6stat.total) { return 0; } - } else - // Can only have one zero if *not* start or end - { - if ((v6stat.zero_pos == 0) || (v6stat.zero_pos == v6stat.total)) { + } else { + // Can only have one zero if *not* start or end + if (v6stat.zero_pos == 0 || v6stat.zero_pos == v6stat.total) { return 0; } } } - // Format result - + // Format the result. if (v6stat.zero_pos >= 0) { // Copy initial part OPENSSL_memcpy(v6, v6stat.tmp, v6stat.zero_pos); @@ -1299,9 +1293,12 @@ // Zero length element, corresponds to '::' if (s->zero_pos == -1) { s->zero_pos = s->total; + } else if (s->zero_pos != s->total) { + // If we've already got a :: its an error + return 0; } - // If we've already got a :: its an error - else if (s->zero_pos != s->total) { + if (s->zero_cnt >= 3) { + // More than three zeros is an error. return 0; } s->zero_cnt++;