From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-34041-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 10912 invoked by alias); 22 Dec 2014 20:36:38 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 34041
Received: (qmail 20268 invoked from network); 22 Dec 2014 20:36:34 -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,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=
	daniel.shahaf.name; h=x-sasl-enc:date:from:to:subject:message-id
	:mime-version:content-type; s=mesmtp; bh=rQx1a+sPxPZYPdx2/8lmfJT
	l+30=; b=2K5lcSwjuHoDDr3IiaK4kmXSFu6wil2e9JfSaX0BUjfb7CdSG+B7csv
	gcOIb46kB5ItyFu452ivD4hpIQwIkO6rK2TlGpkGqZlXt1aLHlL+4jf9QGRCHUfK
	/C8fUdnSfTJvvcfe/dP5je5n/pGOX/0SWX6/kIF1EHkMj/CFULcM=
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=
	messagingengine.com; h=x-sasl-enc:date:from:to:subject
	:message-id:mime-version:content-type; s=smtpout; bh=rQx1a+sPxPZ
	YPdx2/8lmfJTl+30=; b=Bt7MMCBq1wDeassvNYHK7cFlZxFm2HySqtc8Q7nO/DN
	15j88alstG1u4s3lTXgOzvL3L4hvD1cWiPdxFRHuZRlrEuvOX9J3tOiIzqbLHqFA
	kbIEjmrlwOC+4qCuXaV7fI97KUV96GJ/NjKWbGwf3ncSaTn8JQ/BaZlbuKqrZ25k
	=
X-Sasl-enc: K/WPJWHUaMfWB3sMyMO4ZExlmab9vRs5ljNfY5/6RYpl 1419280591
Date: Mon, 22 Dec 2014 20:36:24 +0000
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: zsh-workers@zsh.org
Subject: Insecure tempfile creation
Message-ID: <20141222203624.GA24855@tarsus.local2>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.21 (2010-09-15)

[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 </dev/tty
-${=${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 </dev/tty
+  ${=${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:

tarsus,5:src/zsh% git di
diff --git Functions/Zle/edit-command-line Functions/Zle/edit-command-line
index 250cac6..584ae16 100644
--- Functions/Zle/edit-command-line
+++ Functions/Zle/edit-command-line
@@ -6,7 +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$$
+local tmpfile=`mktemp ${TMPPREFIX:-/tmp/zsh}ecl.XXXXXX`
 
 print -R - "$PREBUFFER$BUFFER" >$tmpfile
 exec </dev/tty

Of course, once we decide what to do with edit-command-line, it would be
good to go through all the other instances and fix them too.

Cheers,

Daniel