From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4150 invoked by alias); 28 Dec 2014 07:45:12 -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: 34067 Received: (qmail 1038 invoked from network); 28 Dec 2014 07:44:58 -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=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=Kc1larcG c=1 sm=1 tr=0 a=FT8er97JFeGWzr5TCOCO5w==:117 a=kj9zAlcOel0A:10 a=q2GGsy2AAAAA:8 a=oR5dmqMzAAAA:8 a=-9mUelKeXuEA:10 a=A92cGCtB03wA:10 a=tq8dSIMlyLIam6w5SIoA:9 a=0znHznEsXtC9OjaT:21 a=q78sVa4LpJjyeyhz:21 a=CjuIK1q_8ugA:10 From: Bart Schaefer Message-id: <141227234421.ZM16038@torch.brasslantern.com> Date: Sat, 27 Dec 2014 23:44:21 -0800 In-reply-to: <141227223029.ZM15959@torch.brasslantern.com> Comments: In reply to Bart Schaefer "Re: Insecure tempfile creation" (Dec 27, 10:30pm) References: <20141222203624.GA24855@tarsus.local2> <141227223029.ZM15959@torch.brasslantern.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: [PATCH] Re: Insecure tempfile creation MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Dec 27, 10:30pm, Bart Schaefer wrote: } } } 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. } } Question related to this: Is there some reason the following does NOT } work? Does zftp fail in the subshell? I found at least one other place where $(zftp ...) was being used, so I presume this is OK. However, somebody (mystical chanting) should check my work in Functions/Zftp/. In the patch below, cases where the use of the temporary file was easily localized or the name didn't matter, I used anonymous functions with an argument created by =(:). Where the name seemed to matter or the usage was scattered, I applied a trick that occurred to me when looking at Functions/Calendar/calendar, to wit: mv -f =(:) tmpfilename The file is created safely and with mode 0600 by =(:) and then renamed to the desired name, which will replace any symlink or other file that has been left in in the way. I then made sure that redirects either used ">>" (knowing the file is empty) or ">|" to avoid issues with noclobber. I think a few files in Zftp may need "emulate zsh" but I did not attempt to fix that. I suppose =(<<<'') would actually be better, since it won't fork. Hm. In compinstall I noticed that a second tempfile was created by adding the digit "2" to a file name already ending in a PID. Since this could collide with the PID of a different shell, I changed it to append "x". This patch does not yet tackle uses of "/tmp" that do not use $TMPPREFIX, nor did I attempt to retrofit zsh-newuser-install, which uses a common prefix name built on $TMPPREFIX for several different files. diff --git a/Completion/Base/Widget/_complete_debug b/Completion/Base/Widget/_complete_debug index 6044863..00f600e 100644 --- a/Completion/Base/Widget/_complete_debug +++ b/Completion/Base/Widget/_complete_debug @@ -9,6 +9,7 @@ local pager w="${(qq)words}" integer debug_fd=-1 { if [[ -t 2 ]]; then + mv -f =(:) $tmp && exec {debug_fd}>&2 2>| $tmp fi diff --git a/Completion/compinstall b/Completion/compinstall index c04543c..7d34ee4 100644 --- a/Completion/compinstall +++ b/Completion/compinstall @@ -1958,11 +1958,14 @@ if [[ -z $ifile || -d $ifile ]] || fi local tmpout=${TMPPREFIX:-/tmp/zsh}compinstall$$ +mv -f =(:) $tmpout && # safe tempfile creation +mv -f =(:) ${tmpout}x || return 1 + # # Assemble the complete set of lines to # insert. # -__ci_output >$tmpout +__ci_output >>$tmpout if [[ -n $ifile ]]; then if [[ $ifile != *(zshrc|zlogin|zshenv) ]]; then @@ -1984,15 +1987,15 @@ future use." fi if { { grep "$endline" $ifile >/dev/null 2>&1 && sed -e "/^[ ]*$endline/r $tmpout -/^[ ]*$startline/,/^[ ]*$endline/d" $ifile >${tmpout}2 } || - { cp $ifile ${tmpout}2 && cat $tmpout >>${tmpout}2 } } && - cp ${tmpout}2 $ifile && rm -f ${tmpout}2; then +/^[ ]*$startline/,/^[ ]*$endline/d" $ifile >>${tmpout}x } || + { cp $ifile ${tmpout}x && cat $tmpout >>${tmpout}x } } && + cp ${tmpout}x $ifile && rm -f ${tmpout}x; then print "\nSuccessfully added compinstall lines to $ifile." rm -f $tmpout else print "\nFailure adding lines to $ifile. Lines left in \`$tmpout'" fi - rm -f ${tmpout}2 + rm -f ${tmpout}x elif read -q key'?Print them to stdout instead ([y]es, [n]o)? '; then cat $tmpout rm -f $tmpout diff --git a/Functions/Calendar/calendar b/Functions/Calendar/calendar index 00f5998..08c4250 100644 --- a/Functions/Calendar/calendar +++ b/Functions/Calendar/calendar @@ -254,8 +254,7 @@ if (( verbose )); then fi local mycmds="${TMPPREFIX:-/tmp/zsh}.calendar_cmds.$$" -touch $mycmds -chmod 600 $mycmds +mv -f =(:) $mycmds # start of subshell for OS file locking ( diff --git a/Functions/Zftp/zfcd_match b/Functions/Zftp/zfcd_match index 95de4c5..2c809c2 100644 --- a/Functions/Zftp/zfcd_match +++ b/Functions/Zftp/zfcd_match @@ -12,7 +12,6 @@ fi local ZFTP_VERBOSE=45 # should we redirect 2>/dev/null or let the user see it? -local tmpf=${TMPPREFIX}zfcm$$ local -a match mbegin mend if [[ $ZFTP_SYSTEM = UNIX* ]]; then @@ -27,9 +26,10 @@ if [[ $ZFTP_SYSTEM = UNIX* ]]; then # If we're using -F, we get away with using a directory # to list, but not a glob. Don't ask me why. reply=(${${(M)${(f)"$(zftp ls -lF $dir)"}:#d*}/(#b)*[[:space:]](*)\//$match[1]}) -# zftp ls -LF $dir >$tmpf -# reply=($(awk '/\/$/ { print substr($1, 1, length($1)-1) }' $tmpf)) -# rm -f $tmpf +# () { +# zftp ls -LF $dir >|$1 +# reply=($(awk '/\/$/ { print substr($1, 1, length($1)-1) }' $1)) +# } =(:) [[ -n $dir && $dir != */ ]] && dir="$dir/" if [[ -n $WIDGET ]]; then _wanted directories expl 'remote directory' \ diff --git a/Functions/Zftp/zfcget b/Functions/Zftp/zfcget index 476a730..4359801 100644 --- a/Functions/Zftp/zfcget +++ b/Functions/Zftp/zfcget @@ -14,7 +14,7 @@ emulate -L zsh [[ $curcontext = :zf* ]] || local curcontext=:zfcget local loc rem stat=0 opt opt_G opt_t remlist locst remst -local tmpfile=${TMPPREFIX}zfcget$$ rstat tsize +local rstat tsize while getopts :Gt opt; do [[ $opt = '?' ]] && print "zfcget: bad option: -$OPTARG" && return 1 @@ -39,10 +39,11 @@ for remlist in $*; do else # Compare the sizes. locst=($(zftp local $loc)) - zftp remote $rem >$tmpfile - rstat=$? - remst=($(<$tmpfile)) - rm -f $tmpfile + () { + zftp remote $rem >|$1 + rstat=$? + remst=($(<$1)) + } =(: temporary file) if [[ $rstat = 2 ]]; then print "Server does not support SIZE command.\n" \ "Assuming you know what you're doing..." 2>&1 diff --git a/Functions/Zftp/zfcput b/Functions/Zftp/zfcput index 85141b6..2cf8fe2 100644 --- a/Functions/Zftp/zfcput +++ b/Functions/Zftp/zfcput @@ -14,7 +14,6 @@ emulate -L zsh [[ $curcontext = :zf* ]] || local curcontext=:zfcput local loc rem stat=0 locst remst offs tailtype -local tmpfile=${TMPPREFIX}zfcget$$ rstat # find how tail works. this is intensely annoying, since it's completely # standard in C. od's no use, since we can only skip whole blocks. @@ -40,10 +39,11 @@ for loc in $*; do else # Compare the sizes. locst=($(zftp local $loc)) - zftp remote $rem >$tmpfile - rstat=$? - remst=($(<$tmpfile)) - rm -f $tmpfile + () { + zftp remote $rem >|$1 + rstat=$? + remst=($(<$1)) + } =(: temporary file) if [[ $rstat = 2 ]]; then print "Server does not support remote status commands.\n" \ "You will have to find out the size by hand and use zftp append." 2>&1 diff --git a/Functions/Zftp/zffcache b/Functions/Zftp/zffcache index 48afdcb..b609c21 100644 --- a/Functions/Zftp/zffcache +++ b/Functions/Zftp/zffcache @@ -19,8 +19,5 @@ fi if [[ $1 = -d ]]; then unset $fcache_name elif (( ${(P)#fcache_name} == 0 )); then - local tmpf=${TMPPREFIX}zffcache$$ - zftp ls >$tmpf - eval "$fcache_name=(\${(f)\"\$(<\$tmpf)\"})" - rm -f $tmpf + eval "$fcache_name=(\${(f)\"\$(zftp ls)\"})" fi diff --git a/Functions/Zftp/zfget_match b/Functions/Zftp/zfget_match index 1d90bea..c2871fa 100644 --- a/Functions/Zftp/zfget_match +++ b/Functions/Zftp/zfget_match @@ -7,22 +7,22 @@ if [[ $1 == $HOME || $1 == $HOME/* ]]; then 1="~${1#$HOME}" fi -local tmpf=${TMPPREFIX}zfgm$$ - if [[ $ZFTP_SYSTEM == UNIX* && $1 == */* ]]; then + setopt localoptions clobber + local tmpf=${TMPPREFIX}zfgm$$ + mv -f =(:) $tmpf + if [[ -n $WIDGET ]]; then local dir=${1:h} [[ $dir = */ ]] || dir="$dir/" zftp ls -LF $dir >$tmpf local reply reply=(${${${(f)"$(<$tmpf)"}##$dir}%\*}) - rm -f $tmpf _wanted files expl 'remote file' compadd -P $dir - $reply else # On the first argument to ls, we usually get away with a glob. zftp ls "$1*$2" >$tmpf reply=($(<$tmpf)) - rm -f $tmpf fi else local fcache_name diff --git a/Functions/Zftp/zfrglob b/Functions/Zftp/zfrglob index 1fb8d76..5015be7 100644 --- a/Functions/Zftp/zfrglob +++ b/Functions/Zftp/zfrglob @@ -33,12 +33,12 @@ if [[ $pat != *[][*?]* && ( -n $zfrglob || $pat != *[(|)#^]* ) ]]; then return 0 fi -local tmpf=${TMPPREFIX}zfrglob$$ if [[ $zfrglob != '' ]]; then - zftp ls "$pat" >$tmpf 2>/dev/null - eval "$1=(\$(<\$tmpf))" - rm -f $tmpf + () { + zftp ls "$pat" >|$1 2>/dev/null + eval "$1=(\$(<\$1))" + } =(: temporary file) else if [[ $ZFTP_SYSTEM = UNIX* && $pat = */* ]]; then # not the current directory and we know how to handle paths @@ -49,10 +49,11 @@ else dir=/ fi nondir=${pat##*/} - zftp ls "$dir" 2>/dev/null >$tmpf - files=($(<$tmpf)) + () { + zftp ls "$dir" 2>/dev/null >|$1 + files=($(<$1)) + } =(: temporary file) files=(${files:t}) - rm -f $tmpf else # we just have to do an ls and hope that's right local fcache_name diff --git a/Functions/Zftp/zftransfer b/Functions/Zftp/zftransfer index c70bf72..432e2f5 100644 --- a/Functions/Zftp/zftransfer +++ b/Functions/Zftp/zftransfer @@ -43,10 +43,11 @@ zfautocheck || return 1 local style zstyle -s ':zftp:zftransfer' progress style if [[ -n $style && $style != none ]]; then - local ZFTP_TSIZE array tmpfile=${TMPPREFIX}zft$$ - zftp remote $file1 >$tmpfile 2>/dev/null - array=($(<$tmpfile)) - rm -f $tmpfile + local ZFTP_TSIZE array + () { + zftp remote $file1 >|$1 2>/dev/null + array=($(<$1)) + } =(: temporary file) [[ $#array -eq 2 ]] && ZFTP_TSIZE=$array[1] fi diff --git a/Functions/Zftp/zftype b/Functions/Zftp/zftype index 0cdf7e2..81f95de 100644 --- a/Functions/Zftp/zftype +++ b/Functions/Zftp/zftype @@ -1,13 +1,11 @@ # function zftype { -local type zftmp=${TMPPREFIX}zftype$$ +local type [[ $curcontext = :zf* ]] || local curcontext=:zftype zfautocheck -d if (( $# == 0 )); then - zftp type >$zftmp - type=$(<$zftmp) - rm -f $zftmp + type=$(zftp type) if [[ $type = I ]]; then print "Current type is image (binary)" return 0 diff --git a/Functions/Zftp/zfuget b/Functions/Zftp/zfuget index c1033c9..7bdaedc 100644 --- a/Functions/Zftp/zfuget +++ b/Functions/Zftp/zfuget @@ -26,7 +26,7 @@ emulate -L zsh [[ $curcontext = :zf* ]] || local curcontext=:zfuget -local loc rem locstats remstats doit tmpfile=${TMPPREFIX}zfuget$$ +local loc rem locstats remstats doit local rstat remlist opt opt_v opt_s opt_G opt_t integer stat do_close @@ -66,12 +66,13 @@ for remlist in $*; do doit=y remstats=() if [[ -f $loc ]]; then - zftp local $loc >$tmpfile - locstats=($(<$tmpfile)) - zftp remote $rem >$tmpfile - rstat=$? - remstats=($(<$tmpfile)) - rm -f $tmpfile + () { + zftp local $loc >|$1 + locstats=($(<$1)) + zftp remote $rem >|$1 + rstat=$? + remstats=($(<$1)) + } =(: temporary file) if [[ $rstat = 2 ]]; then print "Server does not implement full command set required." 1>&2 return 1 diff --git a/Functions/Zftp/zfuput b/Functions/Zftp/zfuput index 4e0e42d..24a3559 100644 --- a/Functions/Zftp/zfuput +++ b/Functions/Zftp/zfuput @@ -12,7 +12,7 @@ emulate -L zsh [[ $curcontext = :zf* ]] || local curcontext=:zfuput -local loc rem locstats remstats doit tmpfile=${TMPPREFIX}zfuput$$ +local loc rem locstats remstats doit local rstat opt opt_v opt_s integer stat do_close @@ -52,12 +52,13 @@ for rem in $*; do stat=1 continue fi - zftp local $loc >$tmpfile - locstats=($(<$tmpfile)) - zftp remote $rem >$tmpfile - rstat=$? - remstats=($(<$tmpfile)) - rm -f $tmpfile + () { + zftp local $loc >|$1 + locstats=($(<$1)) + zftp remote $rem >|$1 + rstat=$? + remstats=($(<$1)) + } =(: temporary file) if [[ $rstat = 2 ]]; then print "Server does not implement full command set required." 1>&2 return 1 diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line index 250cac6..100af96 100644 --- a/Functions/Zle/edit-command-line +++ b/Functions/Zle/edit-command-line @@ -6,12 +6,10 @@ # will give ksh-like behaviour for that key, # except that it will handle multi-line buffers properly. -local tmpfile=${TMPPREFIX:-/tmp/zsh}ecl$$ +() { + exec $tmpfile -exec