From e9ce922ddf2ec6c1095f42ee9857f369084761c3 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Tue, 9 May 2017 15:34:08 -0400
Subject: [PATCH 23/24] efi_loadopt_create(): avoid NULL dereference

covscan rightly points out that dp is allowed to be NULL (and so is
buf), so we can't pass those in to memcpy() in those cases.

So don't.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 src/loadopt.c | 46 +++++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/loadopt.c b/src/loadopt.c
index 5301f3d..cf0886d 100644
--- a/src/loadopt.c
+++ b/src/loadopt.c
@@ -56,36 +56,44 @@ efi_loadopt_create(uint8_t *buf, ssize_t size, uint32_t attributes,
 	}
 
 	if (!buf) {
+invalid:
 		errno = EINVAL;
 		return -1;
 	}
 
-	if (!optional_data && optional_data_size != 0) {
-		errno = EINVAL;
-		return -1;
-	}
+	if (!optional_data && optional_data_size != 0)
+		goto invalid;
 
-	if (!dp && dp_size == 0) {
-		errno = EINVAL;
-		return -1;
-	}
+	if ((!dp && dp_size == 0) || dp_size < 0)
+		goto invalid;
+
+	if (dp) {
+		if (!efidp_is_valid(dp, dp_size))
+			goto invalid;
 
-	uint8_t *pos = buf;
+		if (efidp_size(dp) != dp_size)
+			goto invalid;
+	}
 
-	*(uint32_t *)pos = attributes;
-	pos += sizeof (attributes);
+	if (buf) {
+		uint8_t *pos = buf;
+		*(uint32_t *)pos = attributes;
+		pos += sizeof (attributes);
 
-	*(uint16_t *)pos = dp_size;
-	pos += sizeof (uint16_t);
+		*(uint16_t *)pos = dp_size;
+		pos += sizeof (uint16_t);
 
-	utf8_to_ucs2((uint16_t *)pos, desc_len, 1, (uint8_t *)description);
-	pos += desc_len;
+		utf8_to_ucs2((uint16_t *)pos, desc_len, 1,
+			     (uint8_t *)description);
+		pos += desc_len;
 
-	memcpy(pos, dp, dp_size);
-	pos += dp_size;
+		if (dp)
+			memcpy(pos, dp, dp_size);
+		pos += dp_size;
 
-	if (optional_data && optional_data_size > 0)
-		memcpy(pos, optional_data, optional_data_size);
+		if (optional_data && optional_data_size > 0)
+			memcpy(pos, optional_data, optional_data_size);
+	}
 
 	return sz;
 }
-- 
2.12.2