From a18d80ff9842b413aa1bd8fa17e8c65b7bbb9336 Mon Sep 17 00:00:00 2001 From: Chris Co Date: Tue, 16 Feb 2021 06:21:12 +0000 Subject: sbat: add minor fixes to parse_sbat Add parameter checking to parse_sbat(). Set end pointer to be sbat_base + sbat_size - 1. We directly dereference the end pointer but this is technically outside of our sbat_base buffer range. Remove current and end while loops that account for extra CRLF or LF characters before and after the .sbat section. We will rely on automated tooling to verify the .sbat section is sane. Remove the overwriting of *(end - 1) with '\0'. This behavior causes a segfault in the unit test. parse_sbat_entry() expects a very specific pattern "_,_,_,_,_,_\n" for every entry and uses strchrnul() to process each individual field. When *(end - 1)='\0' is present, it short-circuits the final \n and causes the final get_sbat_field() to return NULL, thereby setting current = NULL. Eventually parse_sbat attempts to access current in the do-while condition and the segfault happens. Signed-off-by: Chris Co --- sbat.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/sbat.c b/sbat.c index 2bddbce2..e996a419 100644 --- a/sbat.c +++ b/sbat.c @@ -70,24 +70,19 @@ error: EFI_STATUS parse_sbat(char *sbat_base, size_t sbat_size, struct sbat *sbat) { - CHAR8 *current = (CHAR8 *)sbat_base; - CHAR8 *end = (CHAR8 *)sbat_base + sbat_size; + CHAR8 *current = (CHAR8 *) sbat_base; + CHAR8 *end = (CHAR8 *) sbat_base + sbat_size - 1; EFI_STATUS efi_status = EFI_SUCCESS; struct sbat_entry *entry; struct sbat_entry **entries; unsigned int i; - while ((*current == '\r' || *current == '\n') && current < end) - current++; + if (!sbat_base || !sbat || sbat_size == 0) + return EFI_INVALID_PARAMETER; if (current == end) return EFI_INVALID_PARAMETER; - while ((*end == '\r' || *end == '\n') && end < current) - end--; - - *(end - 1) = '\0'; - do { entry = NULL; efi_status = parse_sbat_entry(¤t, end, &entry); -- cgit v1.2.3