From nobody Thu Feb 13 10:23:53 2025 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YtrrB0JTjz5nJVQ; Thu, 13 Feb 2025 10:23:54 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Ytrr95k9Hz3b11; Thu, 13 Feb 2025 10:23:53 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1739442233; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=e8Ryq/0fovGWdCGYLvUn94QNKf1ifGJ9GKcqqcKZEFA=; b=CTwbKKqRnAqfhJHd6udor7ZgJAewqqUWQpYCgPGJMV37oHSHliwl8QaIDMqDPWi6VrdBHd OTUh0bPfJyc2vexTz7xXnEn7V8aL8RlnMxGiAjiICTIZCu3OyW71TI2HRLp7glVpe3RoUG sIaqnbeYvSrVBiBzxAj5MJiv2IxXErnGUL8C1sJsrxrHDlnS6wvdlQb8BSt/Wkn30e/LG0 7C4atzst5dcBdLWUxCNqUiCiiiAElUmC2bTYDtPkSmwjdntBdJRKj5J7uYmZX9FKZg4oYy RWGiLzM1EewpyLCpXqChQVsLi7/u/Av2OQt4G6W6tYXBiNiLIQKMhgg+BgnDfA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1739442233; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=e8Ryq/0fovGWdCGYLvUn94QNKf1ifGJ9GKcqqcKZEFA=; b=o6t0g/xOuHhCuonDTClDgK/E2b+InTlDkROsCAhVG5uyATys8KyS1PTZITEeSyMxZsmjMI SJORgd4aWmC1ma0U9d8nsbTWSEQP0N6PO1KT652T8tyBiDB8UEx8xFPjlsGUq989RdtPu+ P+z6aZVA//kQxI12B77jKxmmw1h1CD1A+o2odiKZ/oYZeH+7YHK+3Kuc0dzxpxSOtWhuNZ rcPRpA4w5O2IBPyl9/ju71uhprHzXIrWNNDk0LxAjpEYwAxA2wFcBCbm7kX/2+SRFZFr/Y GDrKbxvuce9yOumr6KSiefVQJYFyHzY6gunbfiknpZq9JRVg5nSgyeKbADCh/Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1739442233; a=rsa-sha256; cv=none; b=OOM6SoL0bnK+LCFhqMxXd4fF9h6TLLF2SjUmZAfM3hLmKSda/TMOYtm8A779q1RA8UncH9 /Itg0qpSOwvZ5ZVk52VUkYfCpH126nURGVo1nour4hr2wf17SplWD+So+zSL9BLZOE7mC/ K54B1OM03x56BVIXceleNnd9ioFZtr57HXJI3fyV4vh7NbGTIzdDEkTr1UpoEdSG0OH3gm 0G2j0pC90SIZjSghAy04ktg9eRCHF9m/HWN2Fhc3Q2mITOJtRZR67H2Q+c5E+D0vqYNOXT WKrTqjr3oZeUWW0192uDljE0EQq5gqxg3eFybY9jqLFi4oGXAUmBOeGUjiY2aA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Ytrr95K4jzhDS; Thu, 13 Feb 2025 10:23:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 51DANrN7030244; Thu, 13 Feb 2025 10:23:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 51DANrQg030241; Thu, 13 Feb 2025 10:23:53 GMT (envelope-from git) Date: Thu, 13 Feb 2025 10:23:53 GMT Message-Id: <202502131023.51DANrQg030241@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Zhenlei Huang Subject: git: dcd7286d9027 - stable/13 - sysctl: Harden sysctl_handle_string() against unterminated string List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: zlei X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: dcd7286d902774428c08b179a72bfdcd4556ec06 Auto-Submitted: auto-generated The branch stable/13 has been updated by zlei: URL: https://cgit.FreeBSD.org/src/commit/?id=dcd7286d902774428c08b179a72bfdcd4556ec06 commit dcd7286d902774428c08b179a72bfdcd4556ec06 Author: Zhenlei Huang AuthorDate: 2025-02-09 17:17:11 +0000 Commit: Zhenlei Huang CommitDate: 2025-02-13 10:23:04 +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 (cherry picked from commit 1951235537fb62150f1bb15dd7e170ac30853d35) (cherry picked from commit 8ca77f9f9ece9d89161d080eee6a1aa706001878) --- 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);