From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id C065615800F for ; Sun, 12 Feb 2023 18:53:14 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 14523E07E2; Sun, 12 Feb 2023 18:53:14 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id CA3FAE07E2 for ; Sun, 12 Feb 2023 18:53:13 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (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) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id F2F9033BE19 for ; Sun, 12 Feb 2023 18:53:12 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 357187D8 for ; Sun, 12 Feb 2023 18:53:11 +0000 (UTC) From: "Sam James" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Sam James" Message-ID: <1676225541.b654697f3d39014e4600ec5f314a7e94a0637c26.sam@gentoo> Subject: [gentoo-commits] proj/gentoo-functions:master commit in: / X-VCS-Repository: proj/gentoo-functions X-VCS-Files: functions.sh X-VCS-Directories: / X-VCS-Committer: sam X-VCS-Committer-Name: Sam James X-VCS-Revision: b654697f3d39014e4600ec5f314a7e94a0637c26 X-VCS-Branch: master Date: Sun, 12 Feb 2023 18:53:11 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-Archives-Salt: ac5c83a6-2bde-46e7-9968-b3a9a733e02e X-Archives-Hash: af8f4e43cc3b5839e6843952cfeefef8 commit: b654697f3d39014e4600ec5f314a7e94a0637c26 Author: Kerin Millar plushkava net> AuthorDate: Sun Feb 12 09:14:13 2023 +0000 Commit: Sam James gentoo org> CommitDate: Sun Feb 12 18:12:21 2023 +0000 URL: https://gitweb.gentoo.org/proj/gentoo-functions.git/commit/?id=b654697f Prevent code injection and naming conflicts in yesno() Currently, the yesno() function works by first trying to match the first argument against a series of patterns that would reasonably indicate a truthful or falseful value. If it fails to do so, it proceeds to treat the argument as if it were a name reference. However, it does so without bothering to check whether its value could be a legal variable name in the first place, which is extremely dangerous! Below is a trivial example of a code injection. $ . ./functions.sh $ untrusted_input='_; echo "you just got pwned"' $ yesno "$untrusted_input" you just got pwned Frankly, I think the name dereferencing is a monumental anti-feature and would urge that it be removed. However, it is beyond my purview to make such a decision. Instead, add a check to ensure that the name appears to be a valid one before calling upon eval. Here is the same test, as conducted against the new implementation. $ yesno "$untrusted_input" $ echo $? 1 $ EINFO_VERBOSE=1 $ yesno "$untrusted_input" * Invalid argument given to yesno (expected a boolean-like or a legal name) Another issue with allowing for name dereferencing is that, by its very nature, it is anethema to the use of the (non-standard) local builtin. Here is a concrete example of the prior implementation producing a bogus warning and returning the wrong exit status value entirely. $ value=yes $ yesno value * $value is not set properly $ echo $? 1 Compare and contrast to the new implementation, which works correctly. $ yesno value $ echo $? 0 This has been accomplished by eradicating the use of local. Now only the first positional parameter and _ are ever assigned to, with the latter variable being used a loop iterator. This is rather unconventional but is, nevertheless, safe. Although _ is a special variable, its value will not normally be affected by the shell until the enclosing compound command has concluded. Further, any conforming implementation of sh(1) is able to use a for loop to assign to it. Signed-off-by: Kerin Millar plushkava.net> Signed-off-by: Sam James gentoo.org> functions.sh | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/functions.sh b/functions.sh index 6ad22f3..7882113 100644 --- a/functions.sh +++ b/functions.sh @@ -47,20 +47,31 @@ eoutdent() # yesno() { - [ -z "$1" ] && return 1 - - case "$1" in - [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1) return 0;; - [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|[Oo][Ff][Ff]|0) return 1;; - esac - - local value= - eval "value=\$${1}" - case "$value" in - [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1) return 0;; - [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|[Oo][Ff][Ff]|0) return 1;; - *) vewarn "\$$1 is not set properly"; return 1;; - esac + for _ in 1 2; do + case $1 in + [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|[Oo][Ff][Ff]|0) + return 1 + ;; + [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1) + return 0 + esac + if [ "$_" -gt 1 ]; then + ! break + else + # Using eval can be very dangerous. Check whether the + # value is a legitimate variable name before proceeding + # to treat it as one. + ( + LC_ALL=C + case $1 in + ''|_|[[:digit:]]*|*[!_[:alnum:]]*) exit 1 + esac + ) || ! break + # Treat the value as a nameref then try again. + eval "set -- \"\$$1\"" + fi + done || vewarn "Invalid argument given to yesno (expected a boolean-like or a legal name)" + return 1 } #