#10 Memory Leak in tls_P_hash

[Anonymous]

Within ssldecode.c in the tls_P_hash function, there is
a loop that looks like this:

while(left){
HMAC_Init(&hm,secret->data,secret->len,md);
HMAC_Update(&hm,A,A_l);
HMAC_Final(&hm,_A,&A_l);
A=_A;

HMAC_Init(&hm,secret->data,secret->len,md);
HMAC_Update(&hm,A,A_l);
HMAC_Update(&hm,seed->data,seed->len);
HMAC_Final(&hm,tmp,&tmp_l);

tocpy=MIN(left,tmp_l);
memcpy(ptr,tmp,tocpy);
ptr+=tocpy;
left-=tocpy;
}

HMAC_cleanup(&hm);

Not that HMAX_Init is called twice for each loop
iteration. But HMAC_cleanup is only called outside of
the loop. This results in a significant memory leak.
Since this is the tls_P_hash function, this leak would
not typically be observed when using Internet Explorer
since it defaults to using SSL 3.0.

Apologies if this has already been identifed and fixed.

Virgil Wall
vwall@xactional.com
Principle Software Engineer
XActional, Inc.

[Anonymous]

Logged In: YES
user_id=260177

This is only a bug with OpenSSL 0.9.7 and later.

The correct code should look like:

<.. snip..>
HMAC_CTX hm;
HMAC_CTX_init(&hm);
while(left)
{
HMAC_Init_ex(&hm,secret->data,secret->len,md,NULL);
<... snip ...>
HMAC_Init_ex(&hm,secret->data,secret->len,md,NULL);
<... snip ...>
}

HMAC_CTX_cleanup();

(HMAC_Init_ex allocates or *reuses* the HMAC context, while
HMAC_Init only allocates new contexts... causing the leak
you've observed.

Cheers

---
Francois Jacques
Software Developer
jacf01@yahoo.fr

[Nobody/Anonymous]

Logged In: NO

Hi,
Thanks for the patch.
There is a small mistake at the end of the patch.
The last line should be : HMAC_CTX_cleanup(&hm);
insteed of : HMAC_CTX_cleanup();

Otherwise, there is an error during the compilation.

Best regards,
Mickaël Garnier
http://www.actilis.net
mgarnier ( at ) actilis dot net