Skip to content

Commit 1e9ff7e

Browse files
kohlernikic
authored andcommitted
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.
1 parent 815a2be commit 1e9ff7e

File tree

3 files changed

+7
-21
lines changed

3 files changed

+7
-21
lines changed

ext/hash/hash.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,11 +1125,7 @@ static zend_object* php_hashcontext_create(zend_class_entry *ce) {
11251125
static void php_hashcontext_dtor(zend_object *obj) {
11261126
php_hashcontext_object *hash = php_hashcontext_from_object(obj);
11271127

1128-
/* Just in case the algo has internally allocated resources */
11291128
if (hash->context) {
1130-
unsigned char *dummy = emalloc(hash->ops->digest_size);
1131-
hash->ops->hash_final(dummy, hash->context);
1132-
efree(dummy);
11331129
efree(hash->context);
11341130
hash->context = NULL;
11351131
}

ext/hash/hash_sha3.c

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -240,38 +240,28 @@ const php_hash_ops php_hash_sha3_##bits##_ops = { \
240240

241241
// ==========================================================================
242242

243-
static int hash_sha3_copy(const void *ops, void *orig_context, void *dest_context)
244-
{
245-
PHP_SHA3_CTX* orig = (PHP_SHA3_CTX*)orig_context;
246-
PHP_SHA3_CTX* dest = (PHP_SHA3_CTX*)dest_context;
247-
memcpy(dest->hashinstance, orig->hashinstance, sizeof(Keccak_HashInstance));
248-
return SUCCESS;
249-
}
250-
251243
#define DECLARE_SHA3_OPS(bits) \
252244
void PHP_SHA3##bits##Init(PHP_SHA3_##bits##_CTX* ctx) { \
253-
ctx->hashinstance = emalloc(sizeof(Keccak_HashInstance)); \
254-
Keccak_HashInitialize_SHA3_##bits((Keccak_HashInstance *)ctx->hashinstance); \
245+
ZEND_ASSERT(sizeof(Keccak_HashInstance) <= sizeof(PHP_SHA3_##bits##_CTX)); \
246+
Keccak_HashInitialize_SHA3_##bits((Keccak_HashInstance *)ctx); \
255247
} \
256248
void PHP_SHA3##bits##Update(PHP_SHA3_##bits##_CTX* ctx, \
257249
const unsigned char* input, \
258250
size_t inputLen) { \
259-
Keccak_HashUpdate((Keccak_HashInstance *)ctx->hashinstance, input, inputLen * 8); \
251+
Keccak_HashUpdate((Keccak_HashInstance *)ctx, input, inputLen * 8); \
260252
} \
261253
void PHP_SHA3##bits##Final(unsigned char* digest, \
262254
PHP_SHA3_##bits##_CTX* ctx) { \
263-
Keccak_HashFinal((Keccak_HashInstance *)ctx->hashinstance, digest); \
264-
efree(ctx->hashinstance); \
265-
ctx->hashinstance = NULL; \
255+
Keccak_HashFinal((Keccak_HashInstance *)ctx, digest); \
266256
} \
267257
const php_hash_ops php_hash_sha3_##bits##_ops = { \
268258
(php_hash_init_func_t) PHP_SHA3##bits##Init, \
269259
(php_hash_update_func_t) PHP_SHA3##bits##Update, \
270260
(php_hash_final_func_t) PHP_SHA3##bits##Final, \
271-
hash_sha3_copy, \
261+
php_hash_copy, \
272262
bits >> 3, \
273263
(1600 - (2 * bits)) >> 3, \
274-
sizeof(PHP_SHA3_##bits##_CTX), \
264+
sizeof(PHP_SHA3_CTX), \
275265
1 \
276266
}
277267

ext/hash/php_hash_sha3.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ typedef struct {
2424
unsigned char state[200]; // 5 * 5 * sizeof(uint64)
2525
uint32_t pos;
2626
#else
27-
void *hashinstance;
27+
unsigned char state[224]; // this must fit a Keccak_HashInstance
2828
#endif
2929
} PHP_SHA3_CTX;
3030

0 commit comments

Comments
 (0)