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 <chrco@microsoft.com>

1 files 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(&current, end, &entry);