From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 691F41D015C; Wed, 2 Oct 2024 13:48:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727876937; cv=none; b=pNBrTs9JEQbMyxBeJwwMDsBf3ut17WzNNo2tecz7jjLl03xUBd74Vy85kjPAPhnufn/Bmvd3nmA5a6+rZQY2pvEmzdbPNR9YGTRXCwEQxGYEZ3qenoaaqif+MbiuKKhHFidKYRV4WNeaSmxCgSwfd1j33Xl3xtVbbeDLWewZXsI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727876937; c=relaxed/simple; bh=ph9WiRJs8a129ktBp/7b4VFk3oMzDKfTFOQXPpxOoxc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=BZXtlpBlmy25QeN0jJbGkChKZbB6sw7xakBlyu25VY9eROsd8KiLdrH2jDji6E7Iq+eo6wZ0E4os/+gFL6XRs02Dc0debisz01MfTPdY/i6R07lB22UbaTbSqnGkDNKBr0EV/AIsQk6aRKc3NbWV6YldGYbp089wqOh7HAetrlA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=z2RMXH7/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="z2RMXH7/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6C899C4CEC2; Wed, 2 Oct 2024 13:48:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1727876937; bh=ph9WiRJs8a129ktBp/7b4VFk3oMzDKfTFOQXPpxOoxc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=z2RMXH7/LGoEDTV4UZEhK9jGmyYY9z4T5OKc2QlP9utBJYcsatwbIRknEok77uFNN WqZC2JXH53Qw4hYbmWSq3pvs80QxCW4q0O4iqrlppjwhkb7p/oJNZ2dp5FaHriKF57 75x+e2WWsOZaQ6cF4+6oA7XM5hQoTOFfE+NWmYQM= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Danilo Krummrich , Jann Horn , Luis Chamberlain Subject: [PATCH 6.11 593/695] firmware_loader: Block path traversal Date: Wed, 2 Oct 2024 14:59:51 +0200 Message-ID: <20241002125846.181841837@linuxfoundation.org> X-Mailer: git-send-email 2.46.2 In-Reply-To: <20241002125822.467776898@linuxfoundation.org> References: <20241002125822.467776898@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 6.11-stable review patch. If anyone has any objections, please let me know. ------------------ From: Jann Horn commit f0e5311aa8022107d63c54e2f03684ec097d1394 upstream. Most firmware names are hardcoded strings, or are constructed from fairly constrained format strings where the dynamic parts are just some hex numbers or such. However, there are a couple codepaths in the kernel where firmware file names contain string components that are passed through from a device or semi-privileged userspace; the ones I could find (not counting interfaces that require root privileges) are: - lpfc_sli4_request_firmware_update() seems to construct the firmware filename from "ModelName", a string that was previously parsed out of some descriptor ("Vital Product Data") in lpfc_fill_vpd() - nfp_net_fw_find() seems to construct a firmware filename from a model name coming from nfp_hwinfo_lookup(pf->hwinfo, "nffw.partno"), which I think parses some descriptor that was read from the device. (But this case likely isn't exploitable because the format string looks like "netronome/nic_%s", and there shouldn't be any *folders* starting with "netronome/nic_". The previous case was different because there, the "%s" is *at the start* of the format string.) - module_flash_fw_schedule() is reachable from the ETHTOOL_MSG_MODULE_FW_FLASH_ACT netlink command, which is marked as GENL_UNS_ADMIN_PERM (meaning CAP_NET_ADMIN inside a user namespace is enough to pass the privilege check), and takes a userspace-provided firmware name. (But I think to reach this case, you need to have CAP_NET_ADMIN over a network namespace that a special kind of ethernet device is mapped into, so I think this is not a viable attack path in practice.) Fix it by rejecting any firmware names containing ".." path components. For what it's worth, I went looking and haven't found any USB device drivers that use the firmware loader dangerously. Cc: stable@vger.kernel.org Reviewed-by: Danilo Krummrich Fixes: abb139e75c2c ("firmware: teach the kernel to load firmware files directly from the filesystem") Signed-off-by: Jann Horn Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/20240828-firmware-traversal-v3-1-c76529c63b5f@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -849,6 +849,26 @@ static void fw_log_firmware_info(const s {} #endif +/* + * Reject firmware file names with ".." path components. + * There are drivers that construct firmware file names from device-supplied + * strings, and we don't want some device to be able to tell us "I would like to + * be sent my firmware from ../../../etc/shadow, please". + * + * Search for ".." surrounded by either '/' or start/end of string. + * + * This intentionally only looks at the firmware name, not at the firmware base + * directory or at symlink contents. + */ +static bool name_contains_dotdot(const char *name) +{ + size_t name_len = strlen(name); + + return strcmp(name, "..") == 0 || strncmp(name, "../", 3) == 0 || + strstr(name, "/../") != NULL || + (name_len >= 3 && strcmp(name+name_len-3, "/..") == 0); +} + /* called from request_firmware() and request_firmware_work_func() */ static int _request_firmware(const struct firmware **firmware_p, const char *name, @@ -869,6 +889,14 @@ _request_firmware(const struct firmware goto out; } + if (name_contains_dotdot(name)) { + dev_warn(device, + "Firmware load for '%s' refused, path contains '..' component\n", + name); + ret = -EINVAL; + goto out; + } + ret = _request_firmware_prepare(&fw, name, device, buf, size, offset, opt_flags); if (ret <= 0) /* error or already assigned */ @@ -946,6 +974,8 @@ out: * @name will be used as $FIRMWARE in the uevent environment and * should be distinctive enough not to be confused with any other * firmware image for this or any other device. + * It must not contain any ".." path components - "foo/bar..bin" is + * allowed, but "foo/../bar.bin" is not. * * Caller must hold the reference count of @device. *