zsh/files module and insecure tempfile creation

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

I'm trying to work through the "attack vector" for the mv -f trick.
The situation is that a script wants to create a plain empty file
with a known name, let's say "/tmp/zsh12345".

If the attacker creates his own plain file named /tmp/zsh12345 then the
"mv -f" will clobber it, so that's not at issue.  Therefore an attack
is possible if the attacker can create a directory (or symlink to a
directory) named /tmp/zsh12345 and writable by the zsh process,
because then mv will put the empty plain file inside that directory.

Next the attacker must be able to swap the directory or symlink with
a symlink to his own target file.  Presumably if he could create it
in the first place, he can swap it.  So we do have an attack that can
clobber any file writable by the zsh user.  Anybody disagree?

Additionally for compinstall and calendar, the files so created are
later read back into the shell as commands, so if the attacker waits
for the right moment to do the swap he can run arbitrary commands as
the user.  This is a serious enough issue that those functions should
simply fail if there is no way to create the tempfile securely.

Anyone want to suggest additional verbosity in those cases?  Here is
the minimal patch.  Is it sufficient for zfget_match to return 1, or
does it also need to mess with $reply?  (And is $reply really declared
local in the correct place in zfget_match?  I didn't change that.)


diff --git a/Completion/Base/Widget/_complete_debug b/Completion/Base/Widget/_complete_debug
index 50fc809..ba3d2b4 100644
--- a/Completion/Base/Widget/_complete_debug
+++ b/Completion/Base/Widget/_complete_debug
@@ -9,7 +9,8 @@ local pager w="${(qq)words}"
 integer debug_fd=-1
 {
   if [[ -t 2 ]]; then
-    mv -f =(<<<'') $tmp &&
+    zmodload -m -F zsh/files b:zf_ln 2>/dev/null &&
+    zf_ln -fn =(<<<'') $tmp &&
     exec {debug_fd}>&2 2>| $tmp
   fi
 
diff --git a/Completion/compinstall b/Completion/compinstall
index ae94993..2f99d27 100644
--- a/Completion/compinstall
+++ b/Completion/compinstall
@@ -3,6 +3,8 @@
 emulate -L zsh
 setopt extendedglob
 
+zmodload -m -F zsh/files b:zf_ln || return 1
+
 local key
 local compcontext=-default-
 
@@ -1958,8 +1960,8 @@ if [[ -z $ifile || -d $ifile ]] ||
 fi
 
 local tmpout=${TMPPREFIX:-/tmp/zsh}compinstall$$
-mv -f =(<<<'') $tmpout &&	# safe tempfile creation
-mv -f =(<<<'') ${tmpout}x || return 1
+zf_ln -fn =(<<<'') $tmpout &&	# safe tempfile creation
+zf_ln -fn =(<<<'') ${tmpout}x || return 1
 
 #
 # Assemble the complete set of lines to
diff --git a/Functions/Calendar/calendar b/Functions/Calendar/calendar
index 39fc431..0d651dc 100644
--- a/Functions/Calendar/calendar
+++ b/Functions/Calendar/calendar
@@ -12,6 +12,7 @@ local -A reply
 
 zmodload -i zsh/datetime || return 1
 zmodload -i zsh/zutil || return 1
+zmodload -m -F zsh/files b:zf_ln || return 1
 
 autoload -Uz calendar_{add,parse,read,scandate,show,lockfiles}
 
@@ -254,7 +255,7 @@ if (( verbose )); then
 fi
 
 local mycmds="${TMPPREFIX:-/tmp/zsh}.calendar_cmds.$$"
-mv -f =(<<<'') $mycmds
+zf_ln -fn =(<<<'') $mycmds || return 1
 
 # start of subshell for OS file locking
 (
diff --git a/Functions/Zftp/zfget_match b/Functions/Zftp/zfget_match
index 3ba06c4..3f2bbf3 100644
--- a/Functions/Zftp/zfget_match
+++ b/Functions/Zftp/zfget_match
@@ -1,6 +1,7 @@
 # function zfget_match {
 
 emulate -L zsh
+zmodload -m -F zsh/files b:zf_ln || return 1
 
 # the zfcd hack:  this may not be necessary here
 if [[ $1 == $HOME || $1 == $HOME/* ]]; then
@@ -10,8 +11,8 @@ fi
 if [[ $ZFTP_SYSTEM == UNIX* && $1 == */* ]]; then
   setopt localoptions clobber
   local tmpf=${TMPPREFIX}zfgm$$
-  mv -f =(<<<'') $tmpf
-	
+  zf_ln -fn =(<<<'') $tmpf || return 1
+
   if [[ -n $WIDGET ]]; then
     local dir=${1:h}
     [[ $dir = */ ]] || dir="$dir/"

-- 
Barton E. Schaefer