The branch main has been updated by jrtc27:
URL:
https://cgit.FreeBSD.org/src/commit/?id=d2ef3774306c54f3999732fd02bdff39c6b4cf2a
commit d2ef3774306c54f3999732fd02bdff39c6b4cf2a
Author: Jessica Clarke <jrt...@freebsd.org>
AuthorDate: 2021-12-22 16:47:23 +0000
Commit: Jessica Clarke <jrt...@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);
}
