diff options
| author | Dmitry Baryshkov <dbaryshkov@gmail.com> | 2020-02-14 01:06:45 +0000 |
|---|---|---|
| committer | Dmitry Baryshkov <dbaryshkov@gmail.com> | 2020-02-14 01:06:45 +0000 |
| commit | 4ea2a39c7d613fb16a23f742a38f8c60171762a1 (patch) | |
| tree | b84d4eb342e6362cffb1a2a28282314f82693a91 | |
| parent | 07394935d097e4492923da64fbfbb7ff5af8a8f1 (diff) | |
| download | nss-hg-4ea2a39c7d613fb16a23f742a38f8c60171762a1.tar.gz | |
Bug 1431940 - remove dereference before NULL check in BLAKE2B code. r=kjacobs
Differential Revision: https://phabricator.services.mozilla.com/D62676
| -rw-r--r-- | gtests/freebl_gtest/blake2b_unittest.cc | 12 | ||||
| -rw-r--r-- | lib/freebl/blake2b.c | 14 |
2 files changed, 18 insertions, 8 deletions
diff --git a/gtests/freebl_gtest/blake2b_unittest.cc b/gtests/freebl_gtest/blake2b_unittest.cc index ac9cca83f..067faca0c 100644 --- a/gtests/freebl_gtest/blake2b_unittest.cc +++ b/gtests/freebl_gtest/blake2b_unittest.cc @@ -113,6 +113,18 @@ TEST_F(Blake2BTests, ContextTest2) { << "BLAKE2B_End failed!"; } +TEST_F(Blake2BTests, NullContextTest) { + SECStatus rv = BLAKE2B_Begin(nullptr); + ASSERT_EQ(SECFailure, rv); + + rv = BLAKE2B_Update(nullptr, kat_data.data(), 128); + ASSERT_EQ(SECFailure, rv); + + std::vector<uint8_t> digest(BLAKE2B512_LENGTH); + rv = BLAKE2B_End(nullptr, digest.data(), nullptr, BLAKE2B512_LENGTH); + ASSERT_EQ(SECFailure, rv); +} + TEST_F(Blake2BTests, CloneTest) { ScopedBLAKE2BContext ctx(BLAKE2B_NewContext()); ScopedBLAKE2BContext cloned_ctx(BLAKE2B_NewContext()); diff --git a/lib/freebl/blake2b.c b/lib/freebl/blake2b.c index b4a0442c9..2f14bfc97 100644 --- a/lib/freebl/blake2b.c +++ b/lib/freebl/blake2b.c @@ -147,9 +147,8 @@ static SECStatus blake2b_Begin(BLAKE2BContext* ctx, uint8_t outlen, const uint8_t* key, size_t keylen) { - PORT_Assert(ctx != NULL); if (!ctx) { - goto failure; + goto failure_noclean; } if (outlen == 0 || outlen > BLAKE2B512_LENGTH) { goto failure; @@ -181,6 +180,7 @@ blake2b_Begin(BLAKE2BContext* ctx, uint8_t outlen, const uint8_t* key, failure: PORT_Memset(ctx, 0, sizeof(*ctx)); +failure_noclean: PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } @@ -218,17 +218,11 @@ SECStatus BLAKE2B_Update(BLAKE2BContext* ctx, const unsigned char* in, unsigned int inlen) { - size_t left = ctx->buflen; - size_t fill = BLAKE2B_BLOCK_LENGTH - left; - /* Nothing to do if there's nothing. */ if (inlen == 0) { return SECSuccess; } - PORT_Assert(ctx != NULL); - PORT_Assert(in != NULL); - PORT_Assert(left <= BLAKE2B_BLOCK_LENGTH); if (!ctx || !in) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; @@ -240,6 +234,10 @@ BLAKE2B_Update(BLAKE2BContext* ctx, const unsigned char* in, return SECFailure; } + size_t left = ctx->buflen; + PORT_Assert(left <= BLAKE2B_BLOCK_LENGTH); + size_t fill = BLAKE2B_BLOCK_LENGTH - left; + if (inlen > fill) { if (ctx->buflen) { /* There's some remaining data in ctx->buf that we have to prepend |