From 6fbff7fc8aabeee2254405f254220bbe8c05c67d Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Fri, 5 Jun 2020 16:26:33 +0200
Subject: [PATCH] crypto-api: always allocate memory when serializing iovec_t

The AEAD iov interface falls back to serializing the input buffers if
the low-level cipher doesn't support scatter/gather encryption.
However, there was a bug in the functions used for the serialization,
which causes memory leaks under a certain condition (i.e. the number
of input buffers is 1).

This patch makes the logic of the functions simpler, by removing a
micro-optimization that tries to minimize the number of calls to
malloc/free.

The original problem was reported by Marius Steffen in:
https://bugzilla.samba.org/show_bug.cgi?id=14399
and the cause was investigated by Alexander Haase in:
https://gitlab.com/gnutls/gnutls/-/merge_requests/1277

Signed-off-by: Daiki Ueno <ueno@gnu.org>
---
 lib/crypto-api.c        | 36 +++++++++++-------------------------
 tests/aead-cipher-vec.c | 33 ++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/lib/crypto-api.c b/lib/crypto-api.c
index 45be64ed1..8524f5ed4 100644
--- a/lib/crypto-api.c
+++ b/lib/crypto-api.c
@@ -891,32 +891,23 @@ gnutls_aead_cipher_encrypt(gnutls_aead_cipher_hd_t handle,
 struct iov_store_st {
 	void *data;
 	size_t size;
-	unsigned allocated;
 };
 
 static void iov_store_free(struct iov_store_st *s)
 {
-	if (s->allocated) {
-		gnutls_free(s->data);
-		s->allocated = 0;
-	}
+	gnutls_free(s->data);
 }
 
 static int iov_store_grow(struct iov_store_st *s, size_t length)
 {
-	if (s->allocated || s->data == NULL) {
-		s->size += length;
-		s->data = gnutls_realloc(s->data, s->size);
-		if (s->data == NULL)
-			return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
-		s->allocated = 1;
-	} else {
-		void *data = s->data;
-		size_t size = s->size + length;
-		s->data = gnutls_malloc(size);
-		memcpy(s->data, data, s->size);
-		s->size += length;
-	}
+	void *data;
+
+	s->size += length;
+	data = gnutls_realloc(s->data, s->size);
+	if (data == NULL)
+		return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+
+	s->data = data;
 	return 0;
 }
 
@@ -926,11 +917,6 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)
 	memset(dst, 0, sizeof(*dst));
 	if (iovcnt == 0) {
 		return 0;
-	} else if (iovcnt == 1) {
-		dst->data = iov[0].iov_base;
-		dst->size = iov[0].iov_len;
-		/* implies: dst->allocated = 0; */
-		return 0;
 	} else {
 		int i;
 		uint8_t *p;
@@ -944,11 +930,11 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)
 
 		p = dst->data;
 		for (i=0;i<iovcnt;i++) {
-			memcpy(p, iov[i].iov_base, iov[i].iov_len);
+			if (iov[i].iov_len > 0)
+				memcpy(p, iov[i].iov_base, iov[i].iov_len);
 			p += iov[i].iov_len;
 		}
 
-		dst->allocated = 1;
 		return 0;
 	}
 }
diff --git a/tests/aead-cipher-vec.c b/tests/aead-cipher-vec.c
index fba9010d9..6a30a35f7 100644
--- a/tests/aead-cipher-vec.c
+++ b/tests/aead-cipher-vec.c
@@ -49,6 +49,7 @@ static void start(const char *name, int algo)
 	giovec_t auth_iov[2];
 	uint8_t tag[64];
 	size_t tag_size = 0;
+	size_t i;
 
 	key.data = key16;
 	key.size = gnutls_cipher_get_key_size(algo);
@@ -82,21 +83,23 @@ static void start(const char *name, int algo)
 	if (ret < 0)
 		fail("gnutls_cipher_init: %s\n", gnutls_strerror(ret));
 
-	ret = gnutls_aead_cipher_encryptv2(ch,
-					   iv.data, iv.size,
-					   auth_iov, 2,
-					   iov, 3,
-					   tag, &tag_size);
-	if (ret < 0)
-		fail("could not encrypt data: %s\n", gnutls_strerror(ret));
-
-	ret = gnutls_aead_cipher_decryptv2(ch,
-					   iv.data, iv.size,
-					   auth_iov, 2,
-					   iov, 3,
-					   tag, tag_size);
-	if (ret < 0)
-		fail("could not decrypt data: %s\n", gnutls_strerror(ret));
+	for (i = 0; i < 2; i++) {
+		ret = gnutls_aead_cipher_encryptv2(ch,
+						   iv.data, iv.size,
+						   auth_iov, 2,
+						   iov, i + 1,
+						   tag, &tag_size);
+		if (ret < 0)
+			fail("could not encrypt data: %s\n", gnutls_strerror(ret));
+
+		ret = gnutls_aead_cipher_decryptv2(ch,
+						   iv.data, iv.size,
+						   auth_iov, 2,
+						   iov, i + 1,
+						   tag, tag_size);
+		if (ret < 0)
+			fail("could not decrypt data: %s\n", gnutls_strerror(ret));
+	}
 
 	gnutls_aead_cipher_deinit(ch);
 }
-- 
2.25.4