Hans Leidekker : bcrypt: Validate hash output length.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: d75f651404028baa23d2591edf222837a167f790
URL:    https://gitlab.winehq.org/wine/wine/-/commit/d75f651404028baa23d2591edf22283...

Author: Hans Leidekker 
Date:   Tue Nov  1 12:09:38 2022 +0100

bcrypt: Validate hash output length.

---

dlls/bcrypt/bcrypt_main.c  | 19 ++++++++++++-------
 dlls/bcrypt/tests/bcrypt.c | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/dlls/bcrypt/bcrypt_main.c b/dlls/bcrypt/bcrypt_main.c
index e2eb8e95057..266c30fa426 100644
--- a/dlls/bcrypt/bcrypt_main.c
+++ b/dlls/bcrypt/bcrypt_main.c
@@ -496,7 +496,7 @@ static NTSTATUS hash_update( struct hash_impl *hash, enum alg_id alg_id, UCHAR *
     return STATUS_SUCCESS;
 }

-static NTSTATUS hash_finish( struct hash_impl *hash, enum alg_id alg_id, UCHAR *output, ULONG size )
+static NTSTATUS hash_finish( struct hash_impl *hash, enum alg_id alg_id, UCHAR *output )
 {
     switch (alg_id)
     {
@@ -920,8 +920,7 @@ static NTSTATUS hash_prepare( struct hash *hash )
         struct hash_impl temp;
         if ((status = hash_init( &temp, hash->alg_id ))) return status;
         if ((status = hash_update( &temp, hash->alg_id, hash->secret, hash->secret_len ))) return status;
-        if ((status = hash_finish( &temp, hash->alg_id, buffer,
-                                   builtin_algorithms[hash->alg_id].hash_length ))) return status;
+        if ((status = hash_finish( &temp, hash->alg_id, buffer ))) return status;
     }
     else memcpy( buffer, hash->secret, hash->secret_len );

@@ -1050,15 +1049,15 @@ static NTSTATUS hash_finalize( struct hash *hash, UCHAR *output, ULONG size )

if (!(hash->flags & HASH_FLAG_HMAC))
     {
-        if ((status = hash_finish( &hash->inner, hash->alg_id, output, size ))) return status;
+        if ((status = hash_finish( &hash->inner, hash->alg_id, output ))) return status;
         if (hash->flags & HASH_FLAG_REUSABLE) return hash_prepare( hash );
         return STATUS_SUCCESS;
     }

hash_length = builtin_algorithms[hash->alg_id].hash_length;
-    if ((status = hash_finish( &hash->inner, hash->alg_id, buffer, hash_length ))) return status;
+    if ((status = hash_finish( &hash->inner, hash->alg_id, buffer ))) return status;
     if ((status = hash_update( &hash->outer, hash->alg_id, buffer, hash_length ))) return status;
-    if ((status = hash_finish( &hash->outer, hash->alg_id, output, size ))) return status;
+    if ((status = hash_finish( &hash->outer, hash->alg_id, output ))) return status;

if (hash->flags & HASH_FLAG_REUSABLE) return hash_prepare( hash );
     return STATUS_SUCCESS;
@@ -1071,7 +1070,7 @@ NTSTATUS WINAPI BCryptFinishHash( BCRYPT_HASH_HANDLE handle, UCHAR *output, ULON
     TRACE( "%p, %p, %lu, %#lx\n", handle, output, size, flags );

if (!hash) return STATUS_INVALID_HANDLE;
-    if (!output) return STATUS_INVALID_PARAMETER;
+    if (!output || size != builtin_algorithms[hash->alg_id].hash_length) return STATUS_INVALID_PARAMETER;

return hash_finalize( hash, output, size );
 }
@@ -1086,8 +1085,14 @@ NTSTATUS WINAPI BCryptHash( BCRYPT_ALG_HANDLE handle, UCHAR *secret, ULONG secre
     TRACE( "%p, %p, %lu, %p, %lu, %p, %lu\n", handle, secret, secret_len, input, input_len, output, output_len );

if (!alg) return STATUS_INVALID_HANDLE;
+    if (!output) return STATUS_INVALID_PARAMETER;

if ((status = hash_create( alg, secret, secret_len, 0, &hash ))) return status;
+    if (output_len != builtin_algorithms[hash->alg_id].hash_length)
+    {
+        hash_destroy( hash );
+        return STATUS_INVALID_PARAMETER;
+    }
     if ((status = hash_update( &hash->inner, hash->alg_id, input, input_len )))
     {
         hash_destroy( hash );
diff --git a/dlls/bcrypt/tests/bcrypt.c b/dlls/bcrypt/tests/bcrypt.c
index d30e8888e49..2035ce346e6 100644
--- a/dlls/bcrypt/tests/bcrypt.c
+++ b/dlls/bcrypt/tests/bcrypt.c
@@ -330,8 +330,23 @@ static void test_hash(const struct hash_test *test)

if (pBCryptHash)
     {
+        ret = pBCryptHash(BCRYPT_SHA1_ALG_HANDLE, NULL, 0, NULL, 0, hash_buf, 20);
+        ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
+        ret = pBCryptHash(BCRYPT_SHA1_ALG_HANDLE, NULL, 0, (UCHAR *)"test", sizeof("test"), NULL, 20);
+        ok(ret == STATUS_INVALID_PARAMETER, "got %#lx\n", ret);
+        ret = pBCryptHash(BCRYPT_SHA1_ALG_HANDLE, NULL, 0, (UCHAR *)"test", sizeof("test"), hash_buf, 21);
+        ok(ret == STATUS_INVALID_PARAMETER, "got %#lx\n", ret);
+        ret = pBCryptHash(BCRYPT_SHA1_ALG_HANDLE, NULL, 0, (UCHAR *)"test", sizeof("test"), hash_buf, 20);
+        ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
+
         ret = BCryptCreateHash(BCRYPT_SHA1_ALG_HANDLE, &hash, NULL, 0, NULL, 0, 0);
         ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
+        ret = BCryptHashData(hash, (UCHAR *)"test", sizeof("test"), 0);
+        ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
+        ret = BCryptFinishHash(hash, hash_buf, 21, 0);
+        ok(ret == STATUS_INVALID_PARAMETER, "got %#lx\n", ret);
+        ret = BCryptFinishHash(hash, hash_buf, 20, 0);
+        ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
         ret = BCryptDestroyHash(hash);
         ok(ret == STATUS_SUCCESS, "got %#lx\n", ret);
     }