From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20089 invoked by alias); 27 Sep 2016 09:12:39 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 39460 Received: (qmail 2666 invoked from network); 27 Sep 2016 09:12:39 -0000 X-Qmail-Scanner-Diagnostics: from mailout1.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.11):SA:0(-3.0/5.0):. Processed in 0.586601 secs); 27 Sep 2016 09:12:39 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=RP_MATCHES_RCVD autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f1-f79f46d0000008eb-1a-57ea35a02519 Date: Tue, 27 Sep 2016 10:02:21 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: BUG: crafting SHELLOPTS and PS4 allows to run arbitrary programs in setuid binaries using system Message-id: <20160927100221.7d4f744f@pwslap01u.europe.root.pri> In-reply-to: <20160927075347.GA500@fujitsu.shahaf.local2> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: quoted-printable X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrJIsWRmVeSWpSXmKPExsWy7djP87oLTF+FG8y5aGNxsPkhkwOjx6qD H5gCGKO4bFJSczLLUov07RK4MtZ1P2cpuK5c0Xhfr4HxoEwXIyeHhICJxM63b5ghbDGJC/fW s3UxcnEICSxllHhxcRuU08sk8eDqUTaYjrtnZ0IlljFK7N65CCwhJDCNSWLXNEuIxBlGiQXX rjNDOGcZJXpW/wZyODhYBFQldh4JA2lgEzCUmLppNiOILSIgLnF27XkWEFtYIF/iZu8esJt4 Bewl1k1tYwNp5RSwlLjUaQkS5hfQl7j69xMTxEH2EjOvnGGEKBeU+DH5HtgYZgFNia2717ND 2NoST95dYAU5R0LgP5tEy8JmVpCZEgKyEpsOMEOYLhIz17pBjBSWeHV8CzuELSPR2XEQalU/ o8STbl+IMTMYJU6f2QENFGuJvtsXGSF28UlM2jYdaiavREebEESJh8TKs21Q5Y4SR461M01g VJyF5OpZSK6eheTqBYzMqxhFUkuLc9NTi430ihNzi0vz0vWS83M3MQITwOl/xz/uYHx/wuoQ owAHoxIPr0X5y3Ah1sSy4srcQ4wSHMxKIryfDF6FC/GmJFZWpRblxxeV5qQWH2KU5mBREufd s+BKuJBAemJJanZqakFqEUyWiYNTqoFRt9Ly+YvLEht7ef8Lfd0guMygPDA6IjV3VhaTr9qH E6+VRf+4PJgXUh0R80WQMXoGT+DN13V8h+LOrLbJjZ7WdzD1S8Tlx7p3GebsYChyLT6bJz/H sLsl/150+27TKdlrl3wWnttXq60axvLQ++GiPg2bZ1IqXjdZlI56uyf43F1yz/Yvn60SS3FG oqEWc1FxIgD/Q/zD/AIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrAIsWRmVeSWpSXmKPExsVy+t/xq7q1pq/CDV7+1LQ42PyQyYHRY9XB D0wBjFFuNhmpiSmpRQqpecn5KZl56bZKoSFuuhZKCnmJuam2ShG6viFBSgpliTmlQJ6RARpw cA5wD1bSt0twy1jX/Zyl4LpyReN9vQbGgzJdjJwcEgImEnfPzmSDsMUkLtxbD2RzcQgJLGGU mLJsFZQzg0liVvsedgjnHKPE3SmzmboYOYCcs4wS8/xATBYBVYmdR8JABrEJGEpM3TSbEcQW ERCXOLv2PAtIibBAvsS/RjuQMK+AvcS6qW1sIGFOAUuJS52WIGEhgauMEtOOp4HY/AL6Elf/ fmKCOM1eYuaVM4wQrYISPybfYwGxmQXUJSbNW8QMYWtLPHl3gRVijrrEjbu72ScwCs9C0jIL ScssJC0LGJlXMYqklhbnpucWG+oVJ+YWl+al6yXn525iBMbOtmM/N+9gvLQx+BCjAAejEg+v RfnLcCHWxLLiytxDjBIczEoivJ8MXoUL8aYkVlalFuXHF5XmpBYfYjQFhspEZinR5HxgXOeV xBuaGJpbGhoZW1iYGxkpifOWfLgSLiSQnliSmp2aWpBaBNPHxMEp1cCoG/ixmW3tHs0PO3a/ Cqn7fdPcvzHOxPT9PtWLBg/MtJNFJu9bmrsu5NULI2bPSZvk88Ii/3/p2r0+zYlXo1tV/rvi ibK4kPLpTqdK29kEHx1fVpCT+UrW7Mfum05J7g9yp7IHaq88/dpD6u0t208tt6b6ydWvP/V8 wsrjp54aexZMXCk3daODEktxRqKhFnNRcSIAWu6rFrMCAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160927090224eucas1p1eb96a6ea37638ebcee6cf04be7c83a37 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160927075540eucas1p117bb3a8f5edc7b140696f570982f8c03 X-RootMTR: 20160927075540eucas1p117bb3a8f5edc7b140696f570982f8c03 References: <20160927075347.GA500@fujitsu.shahaf.local2> On Tue, 27 Sep 2016 07:53:47 +0000 Daniel Shahaf wrote: > Mateusz Lenik wrote on Tue, Sep 27, 2016 at 06:59:18 +0000: > > % gcc -xc - -otest <<< 'int main() { setuid(0); system("/bin/date"); }' > > % sudo chown root:root test > > % sudo chmod 4755 test > > % env -i SHELLOPTS=3Dxtrace PS4=3D'$(id)' ./test > > uid=3D0(root) gid=3D... groups=3D.../bin/date > > Tue Sep 27 08:49:16 CEST 2016 >=20 > I can't reproduce that either either 5.0.7 or latest master, even with > =C2=ABsetopt promptsubst=C2=BB in effect. (Does it reproduce in 'zsh -f'= ?) While I can believe there's a problem here, since there's pretty similar logic around in zsh, SHELLOPTS does nothing with zsh, so I suspect the case is different if it does show up. > > The solution that bash folks implemented is to drop PS4 from env when t= he > > shell is ran as root. >=20 > 34015 (89012cf94ca) stopped importing non-ASCII envvars. There may have > been other changes in this area but I couldn't quickly find them. I don't think we make any special arrangements for import as root, currently. It's straightforward to do so. I've attempted to tidy up the logic to the point where I think I understand it. Does the test "(!getuid() || !geteuid())" make sense or should that be something else? pws diff --git a/Src/params.c b/Src/params.c index 384c30a..a85e5a5 100644 --- a/Src/params.c +++ b/Src/params.c @@ -333,6 +333,7 @@ IPDEF6("TRY_BLOCK_ERROR", &try_errflag, varinteger_gsu), IPDEF6("TRY_BLOCK_INTERRUPT", &try_interrupt, varinteger_gsu), =20 #define IPDEF7(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL},BR((void *)B),GSU(varsc= alar_gsu),0,0,NULL,NULL,NULL,0} +#define IPDEF7R(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL|PM_DONTIMPORT_ROOT},BR(= (void *)B),GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0} #define IPDEF7U(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL|PM_UNSET},BR((void *)B)= ,GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0} IPDEF7("OPTARG", &zoptarg), IPDEF7("NULLCMD", &nullcmd), @@ -345,7 +346,7 @@ IPDEF7("PS2", &prompt2), IPDEF7U("RPS2", &rprompt2), IPDEF7U("RPROMPT2", &rprompt2), IPDEF7("PS3", &prompt3), -IPDEF7("PS4", &prompt4), +IPDEF7R("PS4", &prompt4), IPDEF7("SPROMPT", &sprompt), =20 #define IPDEF8(A,B,C,D) {{NULL,A,D|PM_SCALAR|PM_SPECIAL},BR((void *)B),GSU= (colonarr_gsu),0,0,NULL,C,NULL,0} @@ -689,7 +690,28 @@ split_env_string(char *env, char **name, char **value) } else return 0; } - =20 + +/** + * Check parameter flags to see if parameter shouldn't be imported + * from environment at start. + * + * return 1: don't import: 0: ok to import. + */ +static int dontimport(int flags) +{ + /* If explicitly marked as don't export */ + if (flags & PM_DONTIMPORT) + return 1; + /* If value already exported */ + if (flags & PM_EXPORTED) + return 1; + /* If security issue when exporting as root */ + if ((flags & PM_DONTIMPORT_ROOT) && (!getuid() || !geteuid())) + return 1; + /* OK to import */ + return 0; +} + /* Set up parameter hash table. This will add predefined * * parameter entries as well as setting up parameter table * * entries for environment variables we inherit. */ @@ -781,8 +803,13 @@ createparamtable(void) envp2 =3D environ; *envp2; envp2++) { if (split_env_string(*envp2, &iname, &ivalue)) { if (!idigit(*iname) && isident(iname) && !strchr(iname, '[')) { + /* + * Parameters that aren't already in the parameter table + * aren't special to the shell, so it's always OK to + * import. Otherwise, check parameter flags. + */ if ((!(pm =3D (Param) paramtab->getnode(paramtab, iname)) || - !(pm->node.flags & PM_DONTIMPORT || pm->node.flags & PM_EXPORTED)) = && + !dontimport(pm->node.flags)) && (pm =3D assignsparam(iname, metafy(ivalue, -1, META_DUP), ASSPM_ENV_IMPORT))) { pm->node.flags |=3D PM_EXPORTED; diff --git a/Src/zsh.h b/Src/zsh.h index bb8ce13..052d754 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1802,6 +1802,7 @@ struct tieddata { #define PM_ZSHSTORED (1<<18) /* function stored in zsh form */ =20 /* Remaining flags do not correspond directly to command line arguments */ +#define PM_DONTIMPORT_ROOT (1<<19) /* do not import if running as root */ #define PM_SINGLE (1<<20) /* special can only have a single instance= */ #define PM_LOCAL (1<<21) /* this parameter will be made local */ #define PM_SPECIAL (1<<22) /* special builtin parameter */