From c6bedd5b83529925c3ec08f96a3bf61c81bff0ae Mon Sep 17 00:00:00 2001

From: Laszlo Ersek <lersek@redhat.com>

Date: Tue, 28 Jan 2020 23:33:46 +0100

Subject: [PATCH 44/62] translate_slashes(): don't write to string literals

Currently, all three invocations of the translate_slashes() function may

lead to writes to the string literal that is #defined with the

DEFAULT_LOADER_CHAR macro. According to ISO C99 6.4.5p6, this is undefined

behavior ("If the program attempts to modify such an array, the behavior

is undefined").

This bug crashes shim on e.g. the 64-bit ArmVirtQemu platform ("Data

abort: Permission fault"), where the platform firmware maps the .text

section (which contains the string literal) read-only.

Modify translate_slashes() so that it copies and translates characters

from an input array of "char" to an output array of "CHAR8".

While at it, fix another bug. Before this patch, if translate_slashes()

ever encountered a double backslash (translating it to a single forward

slash), then the output would end up shorter than the input. However, the

output was not NUL-terminated in-place, therefore the original string

length (and according trailing garbage) would be preserved. After this

patch, the NUL-termination on contraction is automatic, as the output

array's contents are indeterminate when entering the function, and so we

must NUL-terminate it anyway.

Fixes: 8e9124227d18475d3bc634c33518963fc8db7c98

Fixes: e62b69a5b0b87c6df7a4fc23906134945309e927

Fixes: 3d79bcb2651b9eae809b975b3e03e2f96c067072

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1795654

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Upstream-commit-id: 9813e8bc8b3

---

 httpboot.c    |  4 ++--

 netboot.c     | 16 +++++++++++-----

 include/str.h | 14 ++++++++------

 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/httpboot.c b/httpboot.c

index 3622e85867c..2d27e8ed993 100644

--- a/httpboot.c

+++ b/httpboot.c

@@ -743,14 +743,14 @@ httpboot_fetch_buffer (EFI_HANDLE image, VOID **buffer, UINT64 *buf_size)

 {

 	EFI_STATUS efi_status;

 	EFI_HANDLE nic;

-	CHAR8 *next_loader = NULL;

+	CHAR8 next_loader[sizeof DEFAULT_LOADER_CHAR];

 	CHAR8 *next_uri = NULL;

 	CHAR8 *hostname = NULL;

 	if (!uri)

 		return EFI_NOT_READY;

-	next_loader = translate_slashes(DEFAULT_LOADER_CHAR);

+	translate_slashes(next_loader, DEFAULT_LOADER_CHAR);

 	/* Create the URI for the next loader based on the original URI */

 	efi_status = generate_next_uri(uri, next_loader, &next_uri);

diff --git a/netboot.c b/netboot.c

index 58babfb4d2e..4922ef284b1 100644

--- a/netboot.c

+++ b/netboot.c

@@ -189,7 +189,9 @@ static BOOLEAN extract_tftp_info(CHAR8 *url)

 	CHAR8 *start, *end;

 	CHAR8 ip6str[40];

 	CHAR8 ip6inv[16];

-	CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR);

+	CHAR8 template[sizeof DEFAULT_LOADER_CHAR];

+

+	translate_slashes(template, DEFAULT_LOADER_CHAR);

 	// to check against str2ip6() errors

 	memset(ip6inv, 0, sizeof(ip6inv));

@@ -254,10 +256,14 @@ static EFI_STATUS parseDhcp6()

 static EFI_STATUS parseDhcp4()

 {

-	CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR);

-	INTN template_len = strlen(template) + 1;

+	CHAR8 template[sizeof DEFAULT_LOADER_CHAR];

+	INTN template_len;

+	UINTN template_ofs = 0;

 	EFI_PXE_BASE_CODE_DHCPV4_PACKET* pkt_v4 = (EFI_PXE_BASE_CODE_DHCPV4_PACKET *)&pxe->Mode->DhcpAck.Dhcpv4;

+	translate_slashes(template, DEFAULT_LOADER_CHAR);

+	template_len = strlen(template) + 1;

+

 	if(pxe->Mode->ProxyOfferReceived) {

 		/*

 		 * Proxy should not have precedence.  Check if DhcpAck

@@ -288,8 +294,8 @@ static EFI_STATUS parseDhcp4()

 			full_path[dir_len-1] = '\0';

 	}

 	if (dir_len == 0 && dir[0] != '/' && template[0] == '/')

-		template++;

-	strcata(full_path, template);

+		template_ofs++;

+	strcata(full_path, template + template_ofs);

 	memcpy(&tftp_addr.v4, pkt_v4->BootpSiAddr, 4);

 	return EFI_SUCCESS;

diff --git a/include/str.h b/include/str.h

index 9a748366bd1..f73c6212cd9 100644

--- a/include/str.h

+++ b/include/str.h

@@ -45,21 +45,23 @@ strcata(CHAR8 *dest, const CHAR8 *src)

 static inline

 __attribute__((unused))

 CHAR8 *

-translate_slashes(char *str)

+translate_slashes(CHAR8 *out, const char *str)

 {

 	int i;

 	int j;

-	if (str == NULL)

-		return (CHAR8 *)str;

+	if (str == NULL || out == NULL)

+		return NULL;

 	for (i = 0, j = 0; str[i] != '\0'; i++, j++) {

 		if (str[i] == '\\') {

-			str[j] = '/';

+			out[j] = '/';

 			if (str[i+1] == '\\')

 				i++;

-		}

+		} else

+			out[j] = str[i];

 	}

-	return (CHAR8 *)str;

+	out[j] = '\0';

+	return out;

 }

 #endif /* SHIM_STR_H */

-- 

2.26.2