[gnutls-devel] [PATCH, RFC] Fix memory leak in wrap_nettle_hash_algorithm().

[Lennert Buytenhek]

wrap_nettle_hash_algorithm() leaks an mpz_t if it is called with
pk == GNUTLS_PK_RSA and sig == NULL, in which case it will return
without going through the regular exit path that clears the mpz_t
it allocated at the beginning of the function.  Use the regular
exit path instead to fix this.

This leak can be triggered via calls to
gnutls_pubkey_get_preferred_hash_algorithm().

Signed-off-by: Lennert Buytenhek <buytenh at wantstofly.org>

--- gnutls-3.3.18/lib/nettle/pk.c.orig	2015-05-23 11:36:13.000000000 +0300
+++ gnutls-3.3.18/lib/nettle/pk.c	2015-10-12 14:47:54.437991944 +0300
@@ -1963,41 +1963,42 @@ static int wrap_nettle_hash_algorithm(gn
 	struct rsa_public_key pub;
 	const mac_entry_st *me;
 	int ret;
 
 	mpz_init(s);
 
 	switch (pk) {
 	case GNUTLS_PK_DSA:
 	case GNUTLS_PK_EC:
 
 		me = _gnutls_dsa_q_to_hash(pk, issuer_params, NULL);
 		if (hash_algo)
 			*hash_algo = (gnutls_digest_algorithm_t)me->id;
 
 		ret = 0;
 		break;
 	case GNUTLS_PK_RSA:
 		if (sig == NULL) {	/* return a sensible algorithm */
 			if (hash_algo)
 				*hash_algo = GNUTLS_DIG_SHA256;
-			return 0;
+			ret = 0;
+			break;
 		}
 
 		_rsa_params_to_pubkey(issuer_params, &pub);
 
 		digest_size = sizeof(digest);
 
 		nettle_mpz_set_str_256_u(s, sig->size, sig->data);
 
 		ret = extract_digest_info(&pub, &di, &rdi, s);
 		if (ret == 0) {
 			ret = GNUTLS_E_PK_SIG_VERIFY_FAILED;
 			gnutls_assert();
 			goto cleanup;
 		}
 
 		digest_size = sizeof(digest);
 		if ((ret =
 		     decode_ber_digest_info(&di, hash_algo, digest,
 					    &digest_size)) < 0) {
 			gnutls_assert();


=== leaktest.c
#include <stdio.h>
#include <gnutls/gnutls.h>
#include <gnutls/abstract.h>

int main(void)
{
	gnutls_x509_privkey_t key;
	int ret;
	gnutls_privkey_t privkey;
	gnutls_pubkey_t pubkey;
	int i;

	gnutls_global_init();

	ret = gnutls_x509_privkey_init(&key);
	if (ret)
		goto err;

	ret = gnutls_x509_privkey_generate(key, GNUTLS_PK_RSA, 512, 0);
	if (ret < 0)
		goto err;

	ret = gnutls_privkey_init(&privkey);
	if (ret < 0)
		goto err;

	ret = gnutls_privkey_import_x509(privkey, key, 0);
	if (ret < 0)
		goto err;

	ret = gnutls_pubkey_init(&pubkey);
	if (ret < 0)
		goto err;

	ret = gnutls_pubkey_import_privkey(pubkey, privkey, 0, 0);
	if (ret < 0)
		goto err;

	for (i = 0; i < 1000; i++) {
		gnutls_digest_algorithm_t dig;

		/*
		 * @@@ FIXME: this leaks memory on gnutls 3.3.18 (8 bytes
		 * per call on x86_64).
		 */
		ret = gnutls_pubkey_get_preferred_hash_algorithm(pubkey,
								 &dig, NULL);
		if (ret < 0)
			goto err;
	}

	gnutls_pubkey_deinit(pubkey);
	gnutls_privkey_deinit(privkey);
	gnutls_x509_privkey_deinit(key);

	gnutls_global_deinit();

	return 0;

err:
	gnutls_perror(ret);

	return 1;
}


=== valgrind output before patch
$ valgrind --leak-check=full ./leaktest 
==20025== Memcheck, a memory error detector
==20025== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==20025== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==20025== Command: ./leaktest
==20025== 
==20025== 
==20025== HEAP SUMMARY:
==20025==     in use at exit: 8,000 bytes in 1,000 blocks
==20025==   total heap usage: 2,928 allocs, 1,928 frees, 152,484 bytes allocated
==20025== 
==20025== 8,000 bytes in 1,000 blocks are definitely lost in loss record 1 of 1
==20025==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20025==    by 0x604A7B8: __gmp_default_allocate (in /usr/lib64/libgmp.so.10.2.0)
==20025==    by 0x6059327: __gmpz_init (in /usr/lib64/libgmp.so.10.2.0)
==20025==    by 0x4F56AC7: wrap_nettle_hash_algorithm (pk.c:1967)
==20025==    by 0x4EEA51A: _gnutls_x509_verify_algorithm (verify.c:1201)
==20025==    by 0x4E84CA4: _gnutls_pk_get_hash_algorithm (gnutls_pk.c:233)
==20025==    by 0x4E9BC7C: gnutls_pubkey_get_preferred_hash_algorithm (gnutls_pubkey.c:270)
==20025==    by 0x40124E: main (in /home/buytenh/tlstest/leaktest)
==20025== 
==20025== LEAK SUMMARY:
==20025==    definitely lost: 8,000 bytes in 1,000 blocks
==20025==    indirectly lost: 0 bytes in 0 blocks
==20025==      possibly lost: 0 bytes in 0 blocks
==20025==    still reachable: 0 bytes in 0 blocks
==20025==         suppressed: 0 bytes in 0 blocks
==20025== 
==20025== For counts of detected and suppressed errors, rerun with: -v
==20025== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)


=== valgrind output after patch
$ valgrind --leak-check=full ./leaktest 
==22747== Memcheck, a memory error detector
==22747== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==22747== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==22747== Command: ./leaktest
==22747== 
==22747== 
==22747== HEAP SUMMARY:
==22747==     in use at exit: 0 bytes in 0 blocks
==22747==   total heap usage: 2,750 allocs, 2,750 frees, 148,677 bytes allocated
==22747== 
==22747== All heap blocks were freed -- no leaks are possible
==22747== 
==22747== For counts of detected and suppressed errors, rerun with: -v
==22747== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)