]> granicus.if.org Git - php/commitdiff
SHA-3 Keccak_Hash: Store Keccak_HashInstance in the main context.
authorEddie Kohler <ekohler@gmail.com>
Mon, 22 Jun 2020 03:08:22 +0000 (23:08 -0400)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 30 Jun 2020 12:26:48 +0000 (14:26 +0200)
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.

ext/hash/hash.c
ext/hash/hash_sha3.c
ext/hash/php_hash_sha3.h

index 31ead3716379582e44a015bf9eb9df5f9a7aa0fd..2d356b613398fb501526bd86813e4ddf8021e7bc 100644 (file)
@@ -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;
        }
index e576adfcbfb946e8f61d6069043f71a1e599b7b2..32621514a8f5e3178fe28ac317e3bfb420acf5b2 100644 (file)
@@ -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 \
 }
 
index 256da7bda61873acfdb8e0841ee02d9f88d9c373..bb392c0ab0943b522fb06a4a269d7e8a400ad4e9 100644 (file)
@@ -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;