From: Eddie Kohler <ekohler@gmail.com>
Date: Mon, 22 Jun 2020 03:08:22 +0000 (-0400)
Subject: SHA-3 Keccak_Hash: Store Keccak_HashInstance in the main context.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1e9ff7e75743432d0676814561875e73ee259036;p=php

SHA-3 Keccak_Hash: Store Keccak_HashInstance in the main context.

Previously, the Keccak_HashInstance was separately allocated.
This could cause memory leaks on errors. For instance,
in php_hash_do_hash_hmac, the following code cleans up after
a file read error:

    if (n < 0) {
    	efree(context);
    	efree(K);
    	zend_string_release(digest);
    	RETURN_FALSE;
    }

This does not call the context's hash_final operation, which
was the only way to free the separately-allocated Keccak state.

The simplest fix is simply to place the Keccak_HashInstance state
inside the context object. Then it doesn't need to be freed.

As a result, there is no need to call hash_final in the
HashContext destructor: HashContexts cannot contain internally
allocated resources.
---

diff --git a/ext/hash/hash.c b/ext/hash/hash.c
index 31ead37163..2d356b6133 100644
--- a/ext/hash/hash.c
+++ b/ext/hash/hash.c
@@ -1125,11 +1125,7 @@ static zend_object* php_hashcontext_create(zend_class_entry *ce) {
 static void php_hashcontext_dtor(zend_object *obj) {
 	php_hashcontext_object *hash = php_hashcontext_from_object(obj);
 
-	/* Just in case the algo has internally allocated resources */
 	if (hash->context) {
-		unsigned char *dummy = emalloc(hash->ops->digest_size);
-		hash->ops->hash_final(dummy, hash->context);
-		efree(dummy);
 		efree(hash->context);
 		hash->context = NULL;
 	}
diff --git a/ext/hash/hash_sha3.c b/ext/hash/hash_sha3.c
index e576adfcbf..32621514a8 100644
--- a/ext/hash/hash_sha3.c
+++ b/ext/hash/hash_sha3.c
@@ -240,38 +240,28 @@ const php_hash_ops php_hash_sha3_##bits##_ops = { \
 
 // ==========================================================================
 
-static int hash_sha3_copy(const void *ops, void *orig_context, void *dest_context)
-{
-	PHP_SHA3_CTX* orig = (PHP_SHA3_CTX*)orig_context;
-	PHP_SHA3_CTX* dest = (PHP_SHA3_CTX*)dest_context;
-	memcpy(dest->hashinstance, orig->hashinstance, sizeof(Keccak_HashInstance));
-	return SUCCESS;
-}
-
 #define DECLARE_SHA3_OPS(bits) \
 void PHP_SHA3##bits##Init(PHP_SHA3_##bits##_CTX* ctx) { \
-	ctx->hashinstance = emalloc(sizeof(Keccak_HashInstance)); \
-	Keccak_HashInitialize_SHA3_##bits((Keccak_HashInstance *)ctx->hashinstance); \
+	ZEND_ASSERT(sizeof(Keccak_HashInstance) <= sizeof(PHP_SHA3_##bits##_CTX)); \
+	Keccak_HashInitialize_SHA3_##bits((Keccak_HashInstance *)ctx); \
 } \
 void PHP_SHA3##bits##Update(PHP_SHA3_##bits##_CTX* ctx, \
                             const unsigned char* input, \
                             size_t inputLen) { \
-	Keccak_HashUpdate((Keccak_HashInstance *)ctx->hashinstance, input, inputLen * 8); \
+	Keccak_HashUpdate((Keccak_HashInstance *)ctx, input, inputLen * 8); \
 } \
 void PHP_SHA3##bits##Final(unsigned char* digest, \
                            PHP_SHA3_##bits##_CTX* ctx) { \
-	Keccak_HashFinal((Keccak_HashInstance *)ctx->hashinstance, digest); \
-	efree(ctx->hashinstance); \
-	ctx->hashinstance = NULL; \
+	Keccak_HashFinal((Keccak_HashInstance *)ctx, digest); \
 } \
 const php_hash_ops php_hash_sha3_##bits##_ops = { \
 	(php_hash_init_func_t) PHP_SHA3##bits##Init, \
 	(php_hash_update_func_t) PHP_SHA3##bits##Update, \
 	(php_hash_final_func_t) PHP_SHA3##bits##Final, \
-	hash_sha3_copy, \
+	php_hash_copy, \
 	bits >> 3, \
 	(1600 - (2 * bits)) >> 3, \
-	sizeof(PHP_SHA3_##bits##_CTX), \
+	sizeof(PHP_SHA3_CTX), \
 	1 \
 }
 
diff --git a/ext/hash/php_hash_sha3.h b/ext/hash/php_hash_sha3.h
index 256da7bda6..bb392c0ab0 100644
--- a/ext/hash/php_hash_sha3.h
+++ b/ext/hash/php_hash_sha3.h
@@ -24,7 +24,7 @@ typedef struct {
 	unsigned char state[200]; // 5 * 5 * sizeof(uint64)
 	uint32_t pos;
 #else
-	void *hashinstance;
+	unsigned char state[224]; // this must fit a Keccak_HashInstance
 #endif
 } PHP_SHA3_CTX;