From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25881 invoked by alias); 22 Dec 2014 22:01:54 -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: 34044 Received: (qmail 685 invoked from network); 22 Dec 2014 22:01:51 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=tStLFS/FbVD24RJjN4V5oJlfLq4J+/K4ujqlD3dM3+I=; b=xkSoU0UDi9zDIrDOWyvh3QVZkg4G0ziXvfe6ly3vMCBX/rtCQ3CA9hIbLXPv+GfR4o cEnYsKpspN0MAqBJHMIOOvTTfS0WM2bXniSv1s29KjjC4O7t+u+10NNa6PnSMhAOoJWO S/jy+6lKsAE8RCeRp/053ISk99ro92gvG71WYI+i8nQ3dXyOraoXvLbSI2LcNQSecHZj Pn8kGME+sLRhGcP2MAlADK4xgSkJWk2YKCW9+Dtc4v/5vt/y/jAUdqofnQYE2St1yLVT RNCXAD0lCjt/x+Cegy64o+HzpvWwtXY8jay90sQzBF2tIkfUYcAWG5U/IFqe6axsueFy YeTQ== MIME-Version: 1.0 X-Received: by 10.50.67.102 with SMTP id m6mr18484572igt.4.1419285706136; Mon, 22 Dec 2014 14:01:46 -0800 (PST) In-Reply-To: <20141222203624.GA24855@tarsus.local2> References: <20141222203624.GA24855@tarsus.local2> Date: Mon, 22 Dec 2014 23:01:46 +0100 Message-ID: Subject: Re: Insecure tempfile creation From: Mikael Magnusson To: Daniel Shahaf Cc: zsh workers Content-Type: text/plain; charset=UTF-8 On Mon, Dec 22, 2014 at 9:36 PM, Daniel Shahaf wrote: > [moving to -workers from private email] > [this mail contains two mutually-exclusive (conflicting) patches] > > Hello. > > A few places in the source distribution use predictable temporary > filenames; for example: > > Completion/Unix/Command/_cvs local d=/tmp/zsh-cvs-work-$$ > Completion/compinstall: local tmpout=${TMPPREFIX:-/tmp/zsh}compinstall$$ > Functions/Calendar/calendar: local mycmds="${TMPPREFIX:-/tmp/zsh}.calendar_cmds.$$" > Functions/Newuser/zsh-newuser-install: local tmpfile=${TMPPREFIX:-/tmp/zsh}-zni-$$ > Functions/Zftp/zfcget: local tmpfile=${TMPPREFIX}zfcget$$ rstat tsize > Functions/Zle/edit-command-line local tmpfile=${TMPPREFIX:-/tmp/zsh}ecl$$ > Test/ztst.zsh: ZTST_in=${TMPPREFIX}.ztst.in.$$ > > Some of these could be vectors for symlink attacks. For example, in the > edit-command-line case, a malicious local user could overwrite an > arbitrary file by creating /tmp/zshecl4242 (where 4242 is the pid of an > interactive zsh run by root) as a symlink and waiting for the user > behind pid 4242 to run edit-command-line. (The attacker would also be > able to see the contents of the edited command line, which is a problem > if it contains passwords.) > > (Paraphrasing Bart:) In general, the "standard library" should create > tempfiles in ${TMPPREFIX:-/tmp}, and take care to explicitly protect > (e.g., via umask settings) any files which need to be private. > > So, for starters: > > diff --git Functions/Zle/edit-command-line Functions/Zle/edit-command-line > index 250cac6..1b1762d 100644 > --- Functions/Zle/edit-command-line > +++ Functions/Zle/edit-command-line > @@ -6,12 +6,16 @@ > # will give ksh-like behaviour for that key, > # except that it will handle multi-line buffers properly. > > -local tmpfile=${TMPPREFIX:-/tmp/zsh}ecl$$ > - > -print -R - "$PREBUFFER$BUFFER" >$tmpfile > -exec -${=${VISUAL:-${EDITOR:-vi}}} $tmpfile > -print -Rz - "$(<$tmpfile)" > - > -command rm -f $tmpfile > -zle send-break # Force reload from the buffer stack > +() { > + # Use =(:) to create a temporary file with 0600 permissions, since > + # the command-line may contain passwords. > + local tmpfile=$1 > + > + print -R - "$PREBUFFER$BUFFER" >$tmpfile > + exec + ${=${VISUAL:-${EDITOR:-vi}}} $tmpfile > + print -Rz - "$(<$tmpfile)" > + > + command rm -f $tmpfile > + zle send-break # Force reload from the buffer stack > +} =(:) > > This causes the tempfile to be generated by gettempname(), which wraps > mktemp(), eliminating the predictable filename. > > Do people find this use of anon functions too obfuscatory? If so, > perhaps a "mktemp" builtin could be added as a thin wrapper around > gettempname() (on systems that don't have a mktemp command already), and > the code be converted to just: Your first patch is not the best version of that solution; --- a/Functions/Zle/edit-command-line +++ b/Functions/Zle/edit-command-line @@ -6,12 +6,7 @@ # will give ksh-like behaviour for that key, # except that it will handle multi-line buffers properly. -local tmpfile=${TMPPREFIX:-/tmp/zsh}ecl$$ - -print -R - "$PREBUFFER$BUFFER" >$tmpfile -exec