From 572234fb6219da9cfe217e1eebf490eb8ff009d1 Mon Sep 17 00:00:00 2001
From: Dmitry Baryshkov <dbaryshkov@gmail.com>
Date: Fri, 14 Feb 2020 01:06:45 +0000
Subject: [PATCH] Bug 1431940 - remove dereference before NULL check in BLAKE2B
 code. r=kjacobs

Differential Revision: https://phabricator.services.mozilla.com/D62676

--HG--
extra : moz-landing-system : lando
---
 gtests/freebl_gtest/blake2b_unittest.cc | 12 ++++++++++++
 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 ac9cca83fd..067faca0ca 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 b4a0442c95..2f14bfc978 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