The branch releng/13.5 has been updated by zlei:
URL:
https://cgit.FreeBSD.org/src/commit/?id=6b0086795db023796af570dccaa942a3071899b6
commit 6b0086795db023796af570dccaa942a3071899b6
Author: Zhenlei Huang <z...@freebsd.org>
AuthorDate: 2025-02-09 17:17:11 +0000
Commit: Zhenlei Huang <z...@freebsd.org>
CommitDate: 2025-02-13 16:52:50 +0000
sysctl: Harden sysctl_handle_string() against unterminated string
In case a variable string which is not null-terminated is passed in,
strlen() may report a length exceeding the max length, hence it is
possible to leak a portion of kernel memory to the userland.
Harden that by using strnlen() to limit the length to the max length.
While here, refactor the code a little to improve readability.
Note that, when calculating the out length, the null terminator '\0' of
the string is taken into account if available. This is not really
necessary but userland applications may have already relied on this
behavior.
Reviewed by: avg, kib, olce
Approved by: re (cperciva)
Fixes: 210176ad76ee sysctl(9): add CTLFLAG_NEEDGIANT flag
MFC after: 4 days
Differential Revision: https://reviews.freebsd.org/D48881
(cherry picked from commit 1951235537fb62150f1bb15dd7e170ac30853d35)
(cherry picked from commit 8ca77f9f9ece9d89161d080eee6a1aa706001878)
(cherry picked from commit dcd7286d902774428c08b179a72bfdcd4556ec06)
---
sys/kern/kern_sysctl.c | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index 371d521d8850..7a4feada157e 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1787,8 +1787,7 @@ int
sysctl_handle_string(SYSCTL_HANDLER_ARGS)
{
char *tmparg;
- size_t outlen;
- int error = 0, ro_string = 0;
+ int error = 0;
/*
* If the sysctl isn't writable and isn't a preallocated tunable that
@@ -1800,33 +1799,32 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)
*/
if ((oidp->oid_kind & (CTLFLAG_WR | CTLFLAG_TUN)) == 0 ||
arg2 == 0 || kdb_active) {
- arg2 = strlen((char *)arg1) + 1;
- ro_string = 1;
- }
+ size_t outlen;
- if (req->oldptr != NULL) {
- if (ro_string) {
- tmparg = arg1;
- outlen = strlen(tmparg) + 1;
- } else {
+ if (arg2 == 0)
+ outlen = arg2 = strlen(arg1) + 1;
+ else
+ outlen = strnlen(arg1, arg2 - 1) + 1;
+
+ tmparg = req->oldptr != NULL ? arg1 : NULL;
+ error = SYSCTL_OUT(req, tmparg, outlen);
+ } else {
+ size_t outlen;
+
+ if (req->oldptr != NULL) {
tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
sx_slock(&sysctlstringlock);
memcpy(tmparg, arg1, arg2);
sx_sunlock(&sysctlstringlock);
- outlen = strlen(tmparg) + 1;
- }
-
- error = SYSCTL_OUT(req, tmparg, outlen);
-
- if (!ro_string)
- free(tmparg, M_SYSCTLTMP);
- } else {
- if (!ro_string)
+ outlen = strnlen(tmparg, arg2 - 1) + 1;
+ } else {
+ tmparg = NULL;
sx_slock(&sysctlstringlock);
- outlen = strlen((char *)arg1) + 1;
- if (!ro_string)
+ outlen = strnlen(arg1, arg2 - 1) + 1;
sx_sunlock(&sysctlstringlock);
- error = SYSCTL_OUT(req, NULL, outlen);
+ }
+ error = SYSCTL_OUT(req, tmparg, outlen);
+ free(tmparg, M_SYSCTLTMP);
}
if (error || !req->newptr)
return (error);
