Skip to content

Commit

Permalink
SHA-3 Keccak_Hash: Store Keccak_HashInstance in the main context.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kohler authored and nikic committed Jun 30, 2020
1 parent 815a2be commit 1e9ff7e
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 21 deletions.
4 changes: 0 additions & 4 deletions ext/hash/hash.c
Expand Up @@ -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;
}
Expand Down
22 changes: 6 additions & 16 deletions ext/hash/hash_sha3.c
Expand Up @@ -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 \
}

Expand Down
2 changes: 1 addition & 1 deletion ext/hash/php_hash_sha3.h
Expand Up @@ -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;

Expand Down

0 comments on commit 1e9ff7e

Please sign in to comment.