From fbc486b50d867450e5b85429d67459022cd95882 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:21 -0400 Subject: Pass the right arguments to EFI_PXE_BASE_CODE_TFTP_READ_FILE A wrong pointer was being passed to EFI_PXE_BASE_CODE_TFTP_READ_FILE, preventing us from getting the file size back from the tftp call, ensuring that we don't have enough information to properly secureboot-validate the retrieved image. --- netboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index c44aeac5..66300d6d 100644 --- a/netboot.c +++ b/netboot.c @@ -326,7 +326,7 @@ EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle) return rc; } -EFI_STATUS FetchNetbootimage(EFI_HANDLE image_handle, VOID **buffer, UINTN *bufsiz) +EFI_STATUS FetchNetbootimage(EFI_HANDLE image_handle, VOID **buffer, UINT64 *bufsiz) { EFI_STATUS rc; EFI_PXE_BASE_CODE_TFTP_OPCODE read = EFI_PXE_BASE_CODE_TFTP_READ_FILE; @@ -344,7 +344,7 @@ EFI_STATUS FetchNetbootimage(EFI_HANDLE image_handle, VOID **buffer, UINTN *bufs try_again: rc = uefi_call_wrapper(pxe->Mtftp, 10, pxe, read, *buffer, overwrite, - &bufsiz, &blksz, &tftp_addr, full_path, NULL, nobuffer); + bufsiz, &blksz, &tftp_addr, full_path, NULL, nobuffer); if (rc == EFI_BUFFER_TOO_SMALL) { /* try again, doubling buf size */ -- cgit v1.2.3 From e2979f2c5fc302fcde557e1f1c16408e17be1b75 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:28 -0400 Subject: Fix nul termination errors in filenames passed to tftp Fix various errors in the tftp string handling, to ensure we always have properly nul-terminated strings. --- netboot.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index 66300d6d..a10b2614 100644 --- a/netboot.c +++ b/netboot.c @@ -53,7 +53,7 @@ static inline unsigned short int __swap16(unsigned short int x) static EFI_PXE_BASE_CODE *pxe; static EFI_IP_ADDRESS tftp_addr; -static char *full_path; +static UINT8 *full_path; typedef struct { @@ -111,7 +111,7 @@ try_again: for (i=0; i < (bs / sizeof(EFI_HANDLE)); i++) { status = uefi_call_wrapper(BS->OpenProtocol, 6, hbuf[i], &pxe_base_code_protocol, - &pxe, image_handle, NULL, + (void **)&pxe, image_handle, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); if (status != EFI_SUCCESS) { @@ -227,15 +227,15 @@ static UINT8 *str2ip6(char *str) static BOOLEAN extract_tftp_info(char *url) { - char *start, *end; + CHAR8 *start, *end; char ip6str[128]; - char *template = "/grubx64.efi"; + CHAR8 *template = (CHAR8 *)"/grubx64.efi"; if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { Print(L"URLS MUST START WITH tftp://\n"); return FALSE; } - start = url + 7; + start = (CHAR8 *)url + 7; if (*start != '[') { Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; @@ -250,21 +250,19 @@ static BOOLEAN extract_tftp_info(char *url) Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; } - *end = '\0'; memset(ip6str, 0, 128); - memcpy(ip6str, start, strlen((UINT8 *)start)); - *end = ']'; + memcpy(ip6str, start, end + 1 - start); end++; memcpy(&tftp_addr.v6, str2ip6(ip6str), 16); - full_path = AllocatePool(strlen((UINT8 *)end)+strlen((UINT8 *)template)+1); + full_path = AllocateZeroPool(strlen(end)+strlen(template)+1); if (!full_path) return FALSE; - memset(full_path, 0, strlen((UINT8 *)end)+strlen((UINT8 *)template)); - memcpy(full_path, end, strlen((UINT8 *)end)); - end = strrchr(full_path, '/'); + memcpy(full_path, end, strlen(end)); + end = (CHAR8 *)strrchr((char *)full_path, '/'); if (!end) - end = full_path; - memcpy(end, template, strlen((UINT8 *)template)); + end = (CHAR8 *)full_path; + memcpy(end, template, strlen(template)); + end[strlen(template)] = '\0'; return TRUE; } @@ -285,19 +283,15 @@ static EFI_STATUS parseDhcp6() static EFI_STATUS parseDhcp4() { - char *template = "/grubx64.efi"; - char *tmp = AllocatePool(16); + CHAR8 *template = (CHAR8 *)"/grubx64.efi"; + full_path = AllocateZeroPool(strlen(template)+1); - - if (!tmp) + if (!full_path) return EFI_OUT_OF_RESOURCES; - memcpy(&tftp_addr.v4, pxe->Mode->DhcpAck.Dhcpv4.BootpSiAddr, 4); - memcpy(tmp, template, 12); - tmp[13] = '\0'; - full_path = tmp; + 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 -- cgit v1.2.3 From 3816832bc5117670426cf291fc01d6ba5bef83d7 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:31 -0400 Subject: Fix an off-by-one error We don't need to add one because our end pointer is already off the end of the string we want to copy. --- netboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index a10b2614..e5433639 100644 --- a/netboot.c +++ b/netboot.c @@ -251,7 +251,7 @@ static BOOLEAN extract_tftp_info(char *url) return FALSE; } memset(ip6str, 0, 128); - memcpy(ip6str, start, end + 1 - start); + memcpy(ip6str, start, end - start); end++; memcpy(&tftp_addr.v6, str2ip6(ip6str), 16); full_path = AllocateZeroPool(strlen(end)+strlen(template)+1); -- cgit v1.2.3 From 6eaa1a9c9e322e7e0b7ef08181c3450655f32857 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:34 -0400 Subject: Misc allocation cleanups --- netboot.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index e5433639..e7010e7a 100644 --- a/netboot.c +++ b/netboot.c @@ -159,10 +159,9 @@ static char *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) if (ntohs(option->OpCode) == 59) { /* This is the bootfile url option */ urllen = ntohs(option->Length); - url = AllocatePool(urllen+2); + url = AllocateZeroPool(urllen+1); if (!url) return NULL; - memset(url, 0, urllen+2); memcpy(url, option->Data, urllen); return url; } @@ -274,10 +273,13 @@ static EFI_STATUS parseDhcp6() bootfile_url = get_v6_bootfile_url(packet); - if (extract_tftp_info(bootfile_url) == FALSE) - return EFI_NOT_FOUND; if (!bootfile_url) return EFI_NOT_FOUND; + if (extract_tftp_info(bootfile_url) == FALSE) { + FreePool(bootfile_url); + return EFI_NOT_FOUND; + } + FreePool(bootfile_url); return EFI_SUCCESS; } -- cgit v1.2.3 From af049ff4578019cc11d04bcfe0707cff5532274a Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:38 -0400 Subject: More consistent types, fewer casts --- netboot.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index e7010e7a..ff63cded 100644 --- a/netboot.c +++ b/netboot.c @@ -141,11 +141,11 @@ try_again: return rc; } -static char *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) +static CHAR8 *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) { void *optr; EFI_DHCP6_PACKET_OPTION *option; - char *url; + CHAR8 *url; UINT32 urllen; optr = pkt->DhcpOptions; @@ -224,7 +224,7 @@ static UINT8 *str2ip6(char *str) return (UINT8 *)ip; } -static BOOLEAN extract_tftp_info(char *url) +static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; char ip6str[128]; @@ -234,7 +234,7 @@ static BOOLEAN extract_tftp_info(char *url) Print(L"URLS MUST START WITH tftp://\n"); return FALSE; } - start = (CHAR8 *)url + 7; + start = url + 7; if (*start != '[') { Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; @@ -269,8 +269,7 @@ static BOOLEAN extract_tftp_info(char *url) static EFI_STATUS parseDhcp6() { EFI_PXE_BASE_CODE_DHCPV6_PACKET *packet = (EFI_PXE_BASE_CODE_DHCPV6_PACKET *)&pxe->Mode->DhcpAck.Raw; - char *bootfile_url; - + CHAR8 *bootfile_url; bootfile_url = get_v6_bootfile_url(packet); if (!bootfile_url) -- cgit v1.2.3 From 69a54db486b6b01c50d6a43294f881cfc9906268 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:47 -0400 Subject: Correct limits on the length of ipv6 addresses The maximum length of a string representation of an ipv6 address is 39 characters (8 groups of 4 hex chars, with 7 colons in between). So don't allocate more room than this - and more importantly, don't blindly accept strings from the server that are longer than our buffer... --- netboot.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index ff63cded..cbbba66a 100644 --- a/netboot.c +++ b/netboot.c @@ -227,7 +227,7 @@ static UINT8 *str2ip6(char *str) static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; - char ip6str[128]; + char ip6str[40]; CHAR8 *template = (CHAR8 *)"/grubx64.efi"; if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { @@ -244,12 +244,16 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) end = start; while ((*end != '\0') && (*end != ']')) { end++; + if (end - start > 39) { + Print(L"TFTP URL includes malformed IPv6 address\n"); + return FALSE; + } } if (end == '\0') { Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); return FALSE; } - memset(ip6str, 0, 128); + memset(ip6str, 0, 40); memcpy(ip6str, start, end - start); end++; memcpy(&tftp_addr.v6, str2ip6(ip6str), 16); -- cgit v1.2.3 From 5ccacd3a48fb8963826a817b42239142e4764914 Mon Sep 17 00:00:00 2001 From: Steve Langasek Date: Tue, 24 Sep 2013 12:05:51 -0400 Subject: Fix a memory leak --- netboot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index cbbba66a..a8904fd8 100644 --- a/netboot.c +++ b/netboot.c @@ -355,6 +355,8 @@ try_again: goto try_again; } + if (rc != EFI_SUCCESS && *buffer) { + FreePool(*buffer); + } return rc; - } -- cgit v1.2.3 From bd145c6082bde9740548ad7f88261008524ede15 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Thu, 26 Sep 2013 11:58:02 -0400 Subject: Define the PXE 2nd stage loader in the beginning of the file Make it easier to change the PXE 2nd stage loader. Conflicts: netboot.c --- netboot.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index a8904fd8..3cb678a0 100644 --- a/netboot.c +++ b/netboot.c @@ -39,6 +39,7 @@ #include "shim.h" #include "netboot.h" +#define DEFAULT_LOADER "/grub.efi" static inline unsigned short int __swap16(unsigned short int x) { @@ -228,7 +229,7 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; char ip6str[40]; - CHAR8 *template = (CHAR8 *)"/grubx64.efi"; + CHAR8 *template = DEFAULT_LOADER; if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { Print(L"URLS MUST START WITH tftp://\n"); @@ -288,7 +289,7 @@ static EFI_STATUS parseDhcp6() static EFI_STATUS parseDhcp4() { - CHAR8 *template = (CHAR8 *)"/grubx64.efi"; + CHAR8 *template = DEFAULT_LOADER; full_path = AllocateZeroPool(strlen(template)+1); if (!full_path) -- cgit v1.2.3 From e053c227014dcaa9c2c9a63bbd00dfe149bfc468 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 26 Sep 2013 11:58:02 -0400 Subject: Since different distros name grub*.efi differently, make it compile-time. Basically, if you don't want grub.efi, you do: make 'DEFAULT_LOADER=\\\\grubx64.efi' Signed-off-by: Peter Jones --- Makefile | 3 +++ netboot.c | 24 ++++++++++++++++++++---- shim.c | 1 - 3 files changed, 23 insertions(+), 5 deletions(-) (limited to 'netboot.c') diff --git a/Makefile b/Makefile index fc4f1313..0e400227 100644 --- a/Makefile +++ b/Makefile @@ -14,9 +14,12 @@ EFI_LIBS = -lefi -lgnuefi --start-group Cryptlib/libcryptlib.a Cryptlib/OpenSSL/ EFI_CRT_OBJS = $(EFI_PATH)/crt0-efi-$(ARCH).o EFI_LDS = elf_$(ARCH)_efi.lds +DEFAULT_LOADER := \\\\grub.efi CFLAGS = -ggdb -O0 -fno-stack-protector -fno-strict-aliasing -fpic \ -fshort-wchar -Wall -Werror -mno-red-zone -maccumulate-outgoing-args \ -mno-mmx -mno-sse \ + "-DDEFAULT_LOADER=L\"$(DEFAULT_LOADER)\"" \ + "-DDEFAULT_LOADER_CHAR=\"$(DEFAULT_LOADER)\"" \ $(EFI_INCLUDES) ifeq ($(ARCH),x86_64) CFLAGS += -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI diff --git a/netboot.c b/netboot.c index 3cb678a0..3554fc82 100644 --- a/netboot.c +++ b/netboot.c @@ -39,8 +39,6 @@ #include "shim.h" #include "netboot.h" -#define DEFAULT_LOADER "/grub.efi" - static inline unsigned short int __swap16(unsigned short int x) { __asm__("xchgb %b0,%h0" @@ -63,6 +61,24 @@ typedef struct { UINT8 Data[1]; } EFI_DHCP6_PACKET_OPTION; +static CHAR8 * +translate_slashes(char *str) +{ + int i; + int j; + if (str == NULL) + return (CHAR8 *)str; + + for (i = 0, j = 0; str[i] != '\0'; i++, j++) { + if (str[i] == '\\') { + str[j] = '/'; + if (str[i+1] == '\\') + i++; + } + } + return (CHAR8 *)str; +} + /* * usingNetboot * Returns TRUE if we identify a protocol that is enabled and Providing us with @@ -229,7 +245,7 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; char ip6str[40]; - CHAR8 *template = DEFAULT_LOADER; + CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR); if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { Print(L"URLS MUST START WITH tftp://\n"); @@ -289,7 +305,7 @@ static EFI_STATUS parseDhcp6() static EFI_STATUS parseDhcp4() { - CHAR8 *template = DEFAULT_LOADER; + CHAR8 *template = (CHAR8 *)DEFAULT_LOADER_CHAR; full_path = AllocateZeroPool(strlen(template)+1); if (!full_path) diff --git a/shim.c b/shim.c index 9b1117f4..82270138 100644 --- a/shim.c +++ b/shim.c @@ -43,7 +43,6 @@ #include "shim_cert.h" #include "ucs2.h" -#define DEFAULT_LOADER L"\\grub.efi" #define FALLBACK L"\\fallback.efi" #define MOK_MANAGER L"\\MokManager.efi" -- cgit v1.2.3 From cb89c25aebe96a7ecaaa3983786e6a7661465b03 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 2 Oct 2013 10:46:26 -0400 Subject: Use CHAR8 not UINT8 for character work. This gets rid of a lot of type casting that we don't need, and helps reduce warnings when I switch a bunch of gnu-efi stuff to taking const arguments. Signed-off-by: Peter Jones --- netboot.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'netboot.c') diff --git a/netboot.c b/netboot.c index 3554fc82..a83c82ae 100644 --- a/netboot.c +++ b/netboot.c @@ -52,7 +52,7 @@ static inline unsigned short int __swap16(unsigned short int x) static EFI_PXE_BASE_CODE *pxe; static EFI_IP_ADDRESS tftp_addr; -static UINT8 *full_path; +static CHAR8 *full_path; typedef struct { @@ -188,10 +188,10 @@ static CHAR8 *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt) return NULL; } -static UINT16 str2ns(UINT8 *str) +static CHAR16 str2ns(CHAR8 *str) { - UINT16 ret = 0; - UINT8 v; + CHAR16 ret = 0; + CHAR8 v; for(;*str;str++) { if ('0' <= *str && *str <= '9') v = *str - '0'; @@ -206,18 +206,18 @@ static UINT16 str2ns(UINT8 *str) return htons(ret); } -static UINT8 *str2ip6(char *str) +static CHAR8 *str2ip6(CHAR8 *str) { UINT8 i, j, p; size_t len; - UINT8 *a, *b, t; + CHAR8 *a, *b, t; static UINT16 ip[8]; for(i=0; i < 8; i++) { ip[i] = 0; } - len = strlen((UINT8 *)str); - a = b = (UINT8 *)str; + len = strlen(str); + a = b = str; for(i=p=0; i < len; i++, b++) { if (*b != ':') continue; @@ -228,7 +228,7 @@ static UINT8 *str2ip6(char *str) if ( *(b+1) == ':' ) break; } - a = b = (UINT8 *)(str + len); + a = b = (str + len); for(j=len, p=7; j > i; j--, a--) { if (*a != ':') continue; @@ -238,13 +238,13 @@ static UINT8 *str2ip6(char *str) *b = t; b = a; } - return (UINT8 *)ip; + return (CHAR8 *)ip; } static BOOLEAN extract_tftp_info(CHAR8 *url) { CHAR8 *start, *end; - char ip6str[40]; + CHAR8 ip6str[40]; CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR); if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) { -- cgit v1.2.3 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 --- include/str.h | 45 +++++++++++++++++++++++++++++++++++++++++++++ netboot.c | 28 +++++++++++++++++++++------- 2 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 include/str.h (limited to 'netboot.c') diff --git a/include/str.h b/include/str.h new file mode 100644 index 00000000..0f3e003a --- /dev/null +++ b/include/str.h @@ -0,0 +1,45 @@ +#ifndef SHIM_STR_H +#define SHIM_STR_H + +static inline +__attribute__((unused)) +unsigned long strnlena(const CHAR8 *s, unsigned long n) +{ + unsigned long i; + for (i = 0; i <= n; i++) + if (s[i] == '\0') + break; + return i; +} + +static inline +__attribute__((unused)) +CHAR8 * +strncpya(CHAR8 *dest, const CHAR8 *src, unsigned long n) +{ + unsigned long i; + + for (i = 0; i < n && src[i] != '\0'; i++) + dest[i] = src[i]; + for (; i < n; i++) + dest[i] = '\0'; + + return dest; +} + +static inline +__attribute__((unused)) +CHAR8 * +strcata(CHAR8 *dest, const CHAR8 *src) +{ + unsigned long dest_len = strlena(dest); + unsigned long i; + + for (i = 0; src[i] != '\0'; i++) + dest[dest_len + i] = src[i]; + dest[dest_len + i] = '\0'; + + return dest; +} + +#endif /* SHIM_STR_H */ 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