util/nvmutil: safer xstrxcmp() - overflow fix

if a points to a buffer shorter than maxlen,
and the string is not null-terminated early,
the loop may read may overflow

e.g.

char buf[3] = {'a', 'b', 'c'};
xstrxcmp(buf, "abc", 50);

this is undefined behaviour, and a bug. C allows
reading past arrays only if the memory exists,
but we can't guarantee that

to fix it, we check the condition for return,
namely NULL character, before using the character
again. This avoids reading further from a multiple
times so we exit as soon as we encounter NULL

this also avoids multiple reads from memory, though
a compiler would optimise that anyway

Signed-off-by: Leah Rowe <leah@libreboot.org>

1 files changed, 10 insertions, 4 deletions

diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c
index 57b223e4..59d2facd 100644
--- a/util/nvmutil/nvmutil.c
+++ b/util/nvmutil/nvmutil.c
@@ -947,11 +947,17 @@ xstrxcmp(const char *a, const char *b, size_t maxlen)
 		err(EINVAL, "Empty string in xstrxcmp");
 
 	for (i = 0; i < maxlen; i++) {
-		if (a[i] != b[i])
-			return (u8)a[i] - (u8)b[i];
+		u8 ac = (u8)a[i];
+		u8 bc = (u8)b[i];
 
-		if (a[i] == '\0')
-			return 0;
+		if (ac == '\0' || bc == '\0') {
+			if (ac == bc)
+				return 0;
+			return ac - bc;
+		}
+
+		if (ac != bc)
+			return ac - bc;
 	}
 
 	/*