git: d2ef3774306c - main - Fix buffer overread in preloaded hostuuid parsing

From: Jessica Clarke <jrtc27_at_FreeBSD.org>
Date: Wed, 22 Dec 2021 16:50:24 UTC
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=d2ef3774306c54f3999732fd02bdff39c6b4cf2a

commit d2ef3774306c54f3999732fd02bdff39c6b4cf2a
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2021-12-22 16:47:23 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2021-12-22 16:47:23 +0000

    Fix buffer overread in preloaded hostuuid parsing
    
    Commit b6be9566d236 stopped prison0_init writing outside of the
    preloaded hostuuid's bounds. However, the preloaded data will not
    (normally) have a NUL in it, and so validate_uuid will walk off the end
    of the buffer in its call to sscanf. Previously if there was any
    whitespace in the string we'd at least know there's a NUL one past the
    end due to the off-by-one error, but now no such byte is guaranteed.
    
    Fix this by copying to a temporary buffer and explicitly adding a NUL.
    
    Whilst here, change the strlcpy call to use a far less suspicious
    argument for dstsize; in practice it's fine, but it's an unusual pattern
    and not necessary.
    
    Found by:       CHERI
    Reviewed by:    emaste, kevans, jhb
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D33616
---
 sys/kern/kern_jail.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index 377539f1d1bd..e505e9bf1276 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -239,6 +239,8 @@ prison0_init(void)
 {
 	uint8_t *file, *data;
 	size_t size;
+	char buf[sizeof(prison0.pr_hostuuid)];
+	bool valid;
 
 	prison0.pr_cpuset = cpuset_ref(thread0.td_cpuset);
 	prison0.pr_osreldate = osreldate;
@@ -258,10 +260,31 @@ prison0_init(void)
 			while (size > 0 && data[size - 1] <= 0x20) {
 				size--;
 			}
-			if (validate_uuid(data, size, NULL, 0) == 0) {
-				(void)strlcpy(prison0.pr_hostuuid, data,
-				    size + 1);
-			} else if (bootverbose) {
+
+			valid = false;
+
+			/*
+			 * Not NUL-terminated when passed from loader, but
+			 * validate_uuid requires that due to using sscanf (as
+			 * does the subsequent strlcpy, since it still reads
+			 * past the given size to return the true length);
+			 * bounce to a temporary buffer to fix.
+			 */
+			if (size >= sizeof(buf))
+				goto done;
+
+			memcpy(buf, data, size);
+			buf[size] = '\0';
+
+			if (validate_uuid(buf, size, NULL, 0) != 0)
+				goto done;
+
+			valid = true;
+			(void)strlcpy(prison0.pr_hostuuid, buf,
+			    sizeof(prison0.pr_hostuuid));
+
+done:
+			if (bootverbose && !valid) {
 				printf("hostuuid: preload data malformed: '%.*s'\n",
 				    (int)size, data);
 			}