Date: Wed, 22 Dec 2021 16:50:24 GMT From: Jessica Clarke <jrtc27@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: d2ef3774306c - main - Fix buffer overread in preloaded hostuuid parsing Message-ID: <202112221650.1BMGoOBY037938@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202112221650.1BMGoOBY037938>