Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 9 Feb 2025 17:23:10 GMT
From:      Zhenlei Huang <zlei@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 1951235537fb - main - sysctl: Harden sysctl_handle_string() against unterminated string
Message-ID:  <202502091723.519HNAM5013082@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch main has been updated by zlei:

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

commit 1951235537fb62150f1bb15dd7e170ac30853d35
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2025-02-09 17:17:11 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2025-02-09 17:17:11 +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
    Fixes:          210176ad76ee sysctl(9): add CTLFLAG_NEEDGIANT flag
    MFC after:      4 days
    Differential Revision:  https://reviews.freebsd.org/D48881
---
 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 da09aaed5558..8cf8e7b19c89 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1885,8 +1885,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
@@ -1898,33 +1897,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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502091723.519HNAM5013082>