From d9355ab635b2e620e6d908317627b7aa9718a999 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 21 Nov 2013 11:48:24 -0500 Subject: Fix path generation for Dhcpv4 bootloader. Right now we always look for e.g. "\grubx64.efi", which is completely wrong. This makes it look for the path shim was loaded from and modify that to end in a sanitized version of our default loader name. Resolves: rhbz#1032583 Signed-off-by: Peter Jones --- netboot.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index a83c82ae..1732dc71 100644 --- a/netboot.c +++ b/netboot.c @@ -38,6 +38,7 @@ #include #include "shim.h" #include "netboot.h" +#include "str.h" static inline unsigned short int __swap16(unsigned short int x) { @@ -305,19 +306,32 @@ static EFI_STATUS parseDhcp6() static EFI_STATUS parseDhcp4() { - CHAR8 *template = (CHAR8 *)DEFAULT_LOADER_CHAR; - full_path = AllocateZeroPool(strlen(template)+1); + CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR); + UINTN template_len = strlen(template) + 1; + + UINTN dir_len = strnlena(pxe->Mode->DhcpAck.Dhcpv4.BootpBootFile, 127); + UINTN i; + UINT8 *dir = pxe->Mode->DhcpAck.Dhcpv4.BootpBootFile; + + for (i = dir_len; i >= 0; i--) { + if (dir[i] == '/') + break; + } + dir_len = (i >= 0) ? i + 1 : 0; + + full_path = AllocateZeroPool(dir_len + template_len); if (!full_path) return EFI_OUT_OF_RESOURCES; + if (dir_len > 0) { + strncpya(full_path, dir, dir_len); + if (full_path[dir_len-1] == '/' && template[0] == '/') + full_path[dir_len-1] = '\0'; + } + strcata(full_path, template); memcpy(&tftp_addr.v4, pxe->Mode->DhcpAck.Dhcpv4.BootpSiAddr, 4); - memcpy(full_path, template, strlen(template)); - - /* Note we don't capture the filename option here because we know its shim.efi - * We instead assume the filename at the end of the path is going to be grubx64.efi - */ return EFI_SUCCESS; } -- cgit v1.2.3 From e724cfb1bf7834c21fbab2baff289e8633633c14 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 21 Nov 2013 11:48:24 -0500 Subject: Lengths that might be -1 can't be unsigned, Peter. Signed-off-by: Peter Jones --- netboot.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index 1732dc71..07e27735 100644 --- a/netboot.c +++ b/netboot.c @@ -307,10 +307,10 @@ static EFI_STATUS parseDhcp6() static EFI_STATUS parseDhcp4() { CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR); - UINTN template_len = strlen(template) + 1; + INTN template_len = strlen(template) + 1; - UINTN dir_len = strnlena(pxe->Mode->DhcpAck.Dhcpv4.BootpBootFile, 127); - UINTN i; + INTN dir_len = strnlena(pxe->Mode->DhcpAck.Dhcpv4.BootpBootFile, 127); + INTN i; UINT8 *dir = pxe->Mode->DhcpAck.Dhcpv4.BootpBootFile; for (i = dir_len; i >= 0; i--) { @@ -329,6 +329,8 @@ static EFI_STATUS parseDhcp4() if (full_path[dir_len-1] == '/' && template[0] == '/') full_path[dir_len-1] = '\0'; } + if (dir_len == 0 && dir[0] != '/' && template[0] == '/') + template++; strcata(full_path, template); memcpy(&tftp_addr.v4, pxe->Mode->DhcpAck.Dhcpv4.BootpSiAddr, 4); -- cgit v1.2.3 From da49ac6d699f2a91bd088fc66ba31c42803ade3e Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Wed, 25 Jun 2014 09:53:23 -0400 Subject: Fetch the netboot image from the same device The previous strategy is to locate the first available PXE_BASE_CODE protocol and to fetch the second stage image from it, and this may cause shim to fetch the wrong second stage image, i.e. grub.efi. Consider the machine with the following boot order: 1. PXE Boot 2. Hard Drive Assume that the EFI image, e.g. bootx64.efi, in the PXE server is broken, then "PXE Boot" will fail and fallback to "Hard Drive". While shim.efi in "Hard Drive" is loaded, it will find the PXE protocol is available and fetch grub.efi from the PXE server, not grub.efi in the disk. This commit checks the DeviceHandle from Loaded Image. If the device supports PXE, then shim fetches grub.efi with the PXE protocol. Otherwise, shim loads grub.efi from the disk. Signed-off-by: Gary Ching-Pang Lin --- netboot.c | 77 +++++++++++++-------------------------------------------------- shim.c | 2 +- 2 files changed, 17 insertions(+), 62 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index 07e27735..5ef53f75 100644 --- a/netboot.c +++ b/netboot.c @@ -85,78 +85,33 @@ translate_slashes(char *str) * Returns TRUE if we identify a protocol that is enabled and Providing us with * the needed information to fetch a grubx64.efi image */ -BOOLEAN findNetboot(EFI_HANDLE image_handle) +BOOLEAN findNetboot(EFI_HANDLE device) { - UINTN bs = sizeof(EFI_HANDLE); - EFI_GUID pxe_base_code_protocol = EFI_PXE_BASE_CODE_PROTOCOL; - EFI_HANDLE *hbuf; - BOOLEAN rc = FALSE; - void *buffer = AllocatePool(bs); - UINTN errcnt = 0; - UINTN i; EFI_STATUS status; - if (!buffer) + status = uefi_call_wrapper(BS->HandleProtocol, 3, device, + &PxeBaseCodeProtocol, (VOID **)&pxe); + if (status != EFI_SUCCESS) { + pxe = NULL; return FALSE; - -try_again: - status = uefi_call_wrapper(BS->LocateHandle,5, ByProtocol, - &pxe_base_code_protocol, NULL, &bs, - buffer); - - if (status == EFI_BUFFER_TOO_SMALL) { - errcnt++; - FreePool(buffer); - if (errcnt > 1) - return FALSE; - buffer = AllocatePool(bs); - if (!buffer) - return FALSE; - goto try_again; } - if (status == EFI_NOT_FOUND) { - FreePool(buffer); + if (!pxe || !pxe->Mode) { + pxe = NULL; return FALSE; } - /* - * We have a list of pxe supporting protocols, lets see if any are - * active - */ - hbuf = buffer; - pxe = NULL; - for (i=0; i < (bs / sizeof(EFI_HANDLE)); i++) { - status = uefi_call_wrapper(BS->OpenProtocol, 6, hbuf[i], - &pxe_base_code_protocol, - (void **)&pxe, image_handle, NULL, - EFI_OPEN_PROTOCOL_GET_PROTOCOL); - - if (status != EFI_SUCCESS) { - pxe = NULL; - continue; - } - - if (!pxe || !pxe->Mode) { - pxe = NULL; - continue; - } - - if (pxe->Mode->Started && pxe->Mode->DhcpAckReceived) { - /* - * We've located a pxe protocol handle thats been - * started and has received an ACK, meaning its - * something we'll be able to get tftp server info - * out of - */ - rc = TRUE; - break; - } - + if (!pxe->Mode->Started || !pxe->Mode->DhcpAckReceived) { + pxe = NULL; + return FALSE; } - FreePool(buffer); - return rc; + /* + * We've located a pxe protocol handle thats been started and has + * received an ACK, meaning its something we'll be able to get + * tftp server info out of + */ + return TRUE; } static CHAR8 *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) diff --git a/shim.c b/shim.c index 48a6f2f1..d8699f98 100644 --- a/shim.c +++ b/shim.c @@ -1373,7 +1373,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) goto done; } - if (findNetboot(image_handle)) { + if (findNetboot(li->DeviceHandle)) { efi_status = parseNetbootinfo(image_handle); if (efi_status != EFI_SUCCESS) { Print(L"Netboot parsing failed: %r\n", efi_status); -- cgit v1.2.3 From f7a182154e19e99e1eb88f5fe8111a37e68cda8e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 12 Aug 2014 10:54:05 -0400 Subject: Factor out x86-isms and add cross compile support This patch cleans up and refactors the Makefiles to better allow new architectures to be added: - remove unused Makefile definitions - import Makefile definitions from top level rather than redefining - move x86 specific CFLAGS to inside ifeq() blocks - remove x86 inline asm - allow $(FORMAT) to be overridden: this is necessary as there exists no EFI or PE/COFF aware objcopy for ARM Signed-off-by: Ard Biesheuvel --- Cryptlib/Makefile | 16 ++++++---------- Cryptlib/OpenSSL/Makefile | 15 ++++++--------- Makefile | 45 +++++++++++++++++++++++++++------------------ lib/Makefile | 14 ++++---------- netboot.c | 10 +--------- 5 files changed, 44 insertions(+), 56 deletions(-) (limited to 'netboot.c') diff --git a/Cryptlib/Makefile b/Cryptlib/Makefile index 678baaca..73a1e2b2 100644 --- a/Cryptlib/Makefile +++ b/Cryptlib/Makefile @@ -1,19 +1,15 @@ -ARCH = $(shell uname -m | sed s,i[3456789]86,ia32,) -EFI_INCLUDE = /usr/include/efi -EFI_INCLUDES = -nostdinc -IInclude -I$(EFI_INCLUDE) -I$(EFI_INCLUDE)/$(ARCH) -I$(EFI_INCLUDE)/protocol -EFI_PATH = /usr/lib64/gnuefi - -LIB_GCC = $(shell $(CC) -print-libgcc-file-name) -EFI_LIBS = -lefi -lgnuefi $(LIB_GCC) +EFI_INCLUDES = -IInclude -I$(EFI_INCLUDE) -I$(EFI_INCLUDE)/$(ARCH) -I$(EFI_INCLUDE)/protocol CFLAGS = -ggdb -O0 -I. -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar \ - -Wall $(EFI_INCLUDES) -mno-red-zone -maccumulate-outgoing-args -mno-sse -mno-mmx + -Wall $(EFI_INCLUDES) + ifeq ($(ARCH),x86_64) - CFLAGS += -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args \ + -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI endif ifeq ($(ARCH),ia32) - CFLAGS += -m32 + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -m32 endif LDFLAGS = -nostdlib -znocombreloc diff --git a/Cryptlib/OpenSSL/Makefile b/Cryptlib/OpenSSL/Makefile index 8e2f2a6f..90975801 100644 --- a/Cryptlib/OpenSSL/Makefile +++ b/Cryptlib/OpenSSL/Makefile @@ -1,19 +1,16 @@ -ARCH = $(shell uname -m | sed s,i[3456789]86,ia32,) -EFI_INCLUDE = /usr/include/efi EFI_INCLUDES = -I../Include -I$(EFI_INCLUDE) -I$(EFI_INCLUDE)/$(ARCH) -I$(EFI_INCLUDE)/protocol -EFI_PATH = /usr/lib64/gnuefi -LIB_GCC = $(shell $(CC) -print-libgcc-file-name) -EFI_LIBS = -lefi -lgnuefi $(LIB_GCC) - -CFLAGS = -ggdb -O0 -I. -I.. -I../Include/ -Icrypto -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar -nostdinc -mno-mmx -mno-sse -mno-red-zone -maccumulate-outgoing-args \ +CFLAGS = -ggdb -O0 -I. -I.. -I../Include/ -Icrypto -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar -nostdinc \ -Wall $(EFI_INCLUDES) -DOPENSSL_SYSNAME_UWIN -DOPENSSL_SYS_UEFI -DL_ENDIAN -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DOPENSSL_NO_CAMELLIA -DOPENSSL_NO_SEED -DOPENSSL_NO_RC5 -DOPENSSL_NO_MDC2 -DOPENSSL_NO_SOCK -DOPENSSL_NO_CMS -DOPENSSL_NO_JPAKE -DOPENSSL_NO_CAPIENG -DOPENSSL_NO_ERR -DOPENSSL_NO_KRB5 -DOPENSSL_NO_DYNAMIC_ENGINE -DGETPID_IS_MEANINGLESS -DOPENSSL_NO_STDIO -DOPENSSL_NO_FP_API -DOPENSSL_NO_DGRAM -DOPENSSL_NO_SHA0 -DOPENSSL_NO_LHASH -DOPENSSL_NO_HW -DOPENSSL_NO_OCSP -DOPENSSL_NO_LOCKING -DOPENSSL_NO_DEPRECATED -DOPENSSL_SMALL_FOOTPRINT -DPEDANTIC + ifeq ($(ARCH),x86_64) - CFLAGS += -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -DSIXTY_FOUR_BIT_LONG + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -maccumulate-outgoing-args \ + -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -DSIXTY_FOUR_BIT_LONG endif ifeq ($(ARCH),ia32) - CFLAGS += -m32 -DTHIRTY_TWO_BIT + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -maccumulate-outgoing-args \ + -m32 -DTHIRTY_TWO_BIT endif LDFLAGS = -nostdlib -znocombreloc diff --git a/Makefile b/Makefile index df190a25..f65bb3b8 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,14 @@ -ARCH = $(shell uname -m | sed s,i[3456789]86,ia32,) +CC = $(CROSS_COMPILE)gcc +LD = $(CROSS_COMPILE)ld +OBJCOPY = $(CROSS_COMPILE)objcopy + +ARCH = $(shell $(CC) -dumpmachine | cut -f1 -d- | sed s,i[3456789]86,ia32,) SUBDIRS = Cryptlib lib LIB_PATH = /usr/lib64 -EFI_INCLUDE = /usr/include/efi +EFI_INCLUDE := /usr/include/efi EFI_INCLUDES = -nostdinc -ICryptlib -ICryptlib/Include -I$(EFI_INCLUDE) -I$(EFI_INCLUDE)/$(ARCH) -I$(EFI_INCLUDE)/protocol -Iinclude EFI_PATH := /usr/lib64/gnuefi @@ -16,9 +20,7 @@ EFI_LDS = elf_$(ARCH)_efi.lds DEFAULT_LOADER := \\\\grub.efi CFLAGS = -ggdb -O0 -fno-stack-protector -fno-strict-aliasing -fpic \ - -fshort-wchar -Wall -Wsign-compare -Werror \ - -mno-red-zone -maccumulate-outgoing-args \ - -mno-mmx -mno-sse -fno-builtin \ + -fshort-wchar -Wall -Wsign-compare -Werror -fno-builtin \ "-DDEFAULT_LOADER=L\"$(DEFAULT_LOADER)\"" \ "-DDEFAULT_LOADER_CHAR=\"$(DEFAULT_LOADER)\"" \ $(EFI_INCLUDES) @@ -26,12 +28,15 @@ CFLAGS = -ggdb -O0 -fno-stack-protector -fno-strict-aliasing -fpic \ ifneq ($(origin OVERRIDE_SECURITY_POLICY), undefined) CFLAGS += -DOVERRIDE_SECURITY_POLICY endif + ifeq ($(ARCH),x86_64) - CFLAGS += -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args \ + -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI endif ifeq ($(ARCH),ia32) - CFLAGS += -m32 + CFLAGS += -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -m32 endif + ifneq ($(origin VENDOR_CERT_FILE), undefined) CFLAGS += -DVENDOR_CERT_FILE=\"$(VENDOR_CERT_FILE)\" endif @@ -95,26 +100,28 @@ MokManager.so: $(MOK_OBJS) Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a $(LD) -o $@ $(LDFLAGS) $^ $(EFI_LIBS) lib/lib.a Cryptlib/libcryptlib.a: - $(MAKE) -C Cryptlib EFI_PATH=$(EFI_PATH) EFI_INCLUDE=$(EFI_INCLUDE) ARCH=$(ARCH) + $(MAKE) -C Cryptlib Cryptlib/OpenSSL/libopenssl.a: - $(MAKE) -C Cryptlib/OpenSSL EFI_PATH=$(EFI_PATH) EFI_INCLUDE=$(EFI_INCLUDE) ARCH=$(ARCH) + $(MAKE) -C Cryptlib/OpenSSL lib/lib.a: - $(MAKE) -C lib EFI_PATH=$(EFI_PATH) EFI_INCLUDE=$(EFI_INCLUDE) ARCH=$(ARCH) + $(MAKE) -C lib + +FORMAT ?= --target efi-app-$(ARCH) %.efi: %.so - objcopy -j .text -j .sdata -j .data \ - -j .dynamic -j .dynsym -j .rel \ - -j .rela -j .reloc -j .eh_frame \ + $(OBJCOPY) -j .text -j .sdata -j .data \ + -j .dynamic -j .dynsym -j .rel* \ + -j .rela* -j .reloc -j .eh_frame \ -j .vendor_cert \ - --target=efi-app-$(ARCH) $^ $@ - objcopy -j .text -j .sdata -j .data \ - -j .dynamic -j .dynsym -j .rel \ - -j .rela -j .reloc -j .eh_frame \ + $(FORMAT) $^ $@ + $(OBJCOPY) -j .text -j .sdata -j .data \ + -j .dynamic -j .dynsym -j .rel* \ + -j .rela* -j .reloc -j .eh_frame \ -j .debug_info -j .debug_abbrev -j .debug_aranges \ -j .debug_line -j .debug_str -j .debug_ranges \ - --target=efi-app-$(ARCH) $^ $@.debug + $(FORMAT) $^ $@.debug %.efi.signed: %.efi certdb/secmod.db pesign -n certdb -i $< -c "shim" -s -o $@ -f @@ -151,3 +158,5 @@ archive: tag @dir=$$PWD; cd /tmp; tar -c --bzip2 -f $$dir/shim-$(VERSION).tar.bz2 shim-$(VERSION) @rm -rf /tmp/shim-$(VERSION) @echo "The archive is in shim-$(VERSION).tar.bz2" + +export ARCH CC LD OBJCOPY EFI_INCLUDE diff --git a/lib/Makefile b/lib/Makefile index a9c9cf66..ebd21a14 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -2,23 +2,17 @@ TARGET = lib.a LIBFILES = simple_file.o guid.o console.o execute.o configtable.o shell.o variables.o security_policy.o -ARCH = $(shell uname -m | sed s,i[3456789]86,ia32,) - -EFI_INCLUDE = /usr/include/efi EFI_INCLUDES = -I$(EFI_INCLUDE) -I$(EFI_INCLUDE)/$(ARCH) -I$(EFI_INCLUDE)/protocol -I../include -EFI_CRT_OBJS = $(EFI_PATH)/crt0-efi-$(ARCH).o -EFI_LDS = $(EFI_PATH)/elf_$(ARCH)_efi.lds - CFLAGS = -ggdb -O0 -fno-stack-protector -fno-strict-aliasing -fpic \ - -fshort-wchar -Wall -mno-red-zone -DBUILD_EFI -fno-builtin \ - -Werror \ + -fshort-wchar -Wall -DBUILD_EFI -fno-builtin -Werror \ $(EFI_INCLUDES) + ifeq ($(ARCH),x86_64) - CFLAGS += -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI + CFLAGS += -mno-red-zone -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI endif ifeq ($(ARCH),ia32) - CFLAGS += -m32 + CFLAGS += -mno-red-zone -m32 endif lib.a: $(LIBFILES) diff --git a/netboot.c b/netboot.c index 5ef53f75..238937d3 100644 --- a/netboot.c +++ b/netboot.c @@ -40,15 +40,7 @@ #include "netboot.h" #include "str.h" -static inline unsigned short int __swap16(unsigned short int x) -{ - __asm__("xchgb %b0,%h0" - : "=q" (x) - : "0" (x)); - return x; -} - -#define ntohs(x) __swap16(x) +#define ntohs(x) __builtin_bswap16(x) /* supported both by GCC and clang */ #define htons(x) ntohs(x) static EFI_PXE_BASE_CODE *pxe; -- cgit v1.2.3 From f6bff34f51cc0ed024ba5262e36a141cd220d4d4 Mon Sep 17 00:00:00 2001 From: Sebastian Krahmer Date: Thu, 2 Oct 2014 01:01:54 -0400 Subject: shim buffer overflow on ipv6 option parsing --- netboot.c | 102 ++++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 40 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index 238937d3..f884cba2 100644 --- a/netboot.c +++ b/netboot.c @@ -108,29 +108,34 @@ BOOLEAN findNetboot(EFI_HANDLE device) static CHAR8 *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) { - void *optr; - EFI_DHCP6_PACKET_OPTION *option; - CHAR8 *url; - UINT32 urllen; + void *optr = NULL, *end = NULL; + EFI_DHCP6_PACKET_OPTION *option = NULL; + CHAR8 *url = NULL; + UINT32 urllen = 0; optr = pkt->DhcpOptions; + end = optr + sizeof(pkt->DhcpOptions); - for(;;) { + for (;;) { option = (EFI_DHCP6_PACKET_OPTION *)optr; if (ntohs(option->OpCode) == 0) - return NULL; + break; if (ntohs(option->OpCode) == 59) { /* This is the bootfile url option */ urllen = ntohs(option->Length); - url = AllocateZeroPool(urllen+1); + if ((void *)(option->Data + urllen) > end) + break; + url = AllocateZeroPool(urllen + 1); if (!url) - return NULL; + break; memcpy(url, option->Data, urllen); return url; } optr += 4 + ntohs(option->Length); + if (optr + sizeof(EFI_DHCP6_PACKET_OPTION) > end) + break; } return NULL; @@ -156,45 +161,60 @@ static CHAR16 str2ns(CHAR8 *str) static CHAR8 *str2ip6(CHAR8 *str) { - UINT8 i, j, p; - size_t len; - CHAR8 *a, *b, t; - static UINT16 ip[8]; + UINT8 i = 0, j = 0, p = 0; + size_t len = 0, dotcount = 0; + enum { MAX_IP6_DOTS = 7 }; + CHAR8 *a = NULL, *b = NULL, t = 0; + static UINT16 ip[8]; - for(i=0; i < 8; i++) { - ip[i] = 0; - } - len = strlen(str); - a = b = str; - for(i=p=0; i < len; i++, b++) { - if (*b != ':') - continue; - *b = '\0'; - ip[p++] = str2ns(a); - *b = ':'; - a = b + 1; - if ( *(b+1) == ':' ) - break; - } - a = b = (str + len); - for(j=len, p=7; j > i; j--, a--) { - if (*a != ':') - continue; - t = *b; - *b = '\0'; - ip[p--] = str2ns(a+1); - *b = t; - b = a; - } - return (CHAR8 *)ip; + memset(ip, 0, sizeof(ip)); + + /* Count amount of ':' to prevent overflows. + * max. count = 7. Returns an invalid ip6 that + * can be checked against + */ + for (a = str; *a != 0; ++a) { + if (*a == ':') + ++dotcount; + } + if (dotcount > MAX_IP6_DOTS) + return (CHAR8 *)ip; + + len = strlen(str); + a = b = str; + for (i = p = 0; i < len; i++, b++) { + if (*b != ':') + continue; + *b = '\0'; + ip[p++] = str2ns(a); + *b = ':'; + a = b + 1; + if (b[1] == ':' ) + break; + } + a = b = (str + len); + for (j = len, p = 7; j > i; j--, a--) { + if (*a != ':') + continue; + t = *b; + *b = '\0'; + ip[p--] = str2ns(a+1); + *b = t; + b = a; + } + return (CHAR8 *)ip; } static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; CHAR8 ip6str[40]; + CHAR8 ip6inv[16]; CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR); + // to check against str2ip6() errors + memset(ip6inv, 0, sizeof(ip6inv)); + if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { Print(L"URLS MUST START WITH tftp://\n"); return FALSE; @@ -209,7 +229,7 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) end = start; while ((*end != '\0') && (*end != ']')) { end++; - if (end - start > 39) { + if (end - start >= (int)sizeof(ip6str)) { Print(L"TFTP URL includes malformed IPv6 address\n"); return FALSE; } @@ -218,10 +238,12 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; } - memset(ip6str, 0, 40); + memset(ip6str, 0, sizeof(ip6str)); memcpy(ip6str, start, end - start); end++; memcpy(&tftp_addr.v6, str2ip6(ip6str), 16); + if (memcmp(&tftp_addr.v6, ip6inv, sizeof(ip6inv)) == 0) + return FALSE; full_path = AllocateZeroPool(strlen(end)+strlen(template)+1); if (!full_path) return FALSE; -- cgit v1.2.3 From 159609ee4eab766673606f5a4e122c78dea390b3 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Oct 2014 01:01:54 -0400 Subject: Correctly reject bad tftp addresses earlier, rather than later. This check is for end == NULL but was meant to be *end == '\0'. Without this change, we'll pass a plausibly bad address (i.e. one with no ']' at the end) to Mtftp(... READ_FILE ...), which should fail correctly, but our error messaging will be inconsistent. Signed-off-by: Peter Jones --- netboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index f884cba2..ad5d37e9 100644 --- a/netboot.c +++ b/netboot.c @@ -234,7 +234,7 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) return FALSE; } } - if (end == '\0') { + if (*end == '\0') { Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; } -- cgit v1.2.3