* zsh/files module and insecure tempfile creation
@ 2015-01-10 6:31 Bart Schaefer
2015-01-10 19:58 ` Peter Stephenson
2015-01-12 5:43 ` Daniel Shahaf
0 siblings, 2 replies; 5+ messages in thread
From: Bart Schaefer @ 2015-01-10 6:31 UTC (permalink / raw)
To: zsh-workers
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: zsh/files module and insecure tempfile creation
2015-01-10 6:31 zsh/files module and insecure tempfile creation Bart Schaefer
@ 2015-01-10 19:58 ` Peter Stephenson
2015-01-11 2:04 ` Bart Schaefer
2015-01-12 5:43 ` Daniel Shahaf
1 sibling, 1 reply; 5+ messages in thread
From: Peter Stephenson @ 2015-01-10 19:58 UTC (permalink / raw)
Cc: zsh-workers
On Fri, 09 Jan 2015 22:31:50 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> +zmodload -m -F zsh/files b:zf_ln || return 1
It's harmless, but you don't need the -m here: there's no pattern to
match.
pws
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: zsh/files module and insecure tempfile creation
2015-01-10 19:58 ` Peter Stephenson
@ 2015-01-11 2:04 ` Bart Schaefer
0 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2015-01-11 2:04 UTC (permalink / raw)
To: zsh-workers
On Jan 10, 7:58pm, Peter Stephenson wrote:
}
} > +zmodload -m -F zsh/files b:zf_ln || return 1
}
} It's harmless, but you don't need the -m here: there's no pattern to
} match.
Ugh. Duh. Didn't edit enough out of the example I modified.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: zsh/files module and insecure tempfile creation
2015-01-10 6:31 zsh/files module and insecure tempfile creation Bart Schaefer
2015-01-10 19:58 ` Peter Stephenson
@ 2015-01-12 5:43 ` Daniel Shahaf
2015-01-12 6:56 ` Bart Schaefer
1 sibling, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2015-01-12 5:43 UTC (permalink / raw)
To: Bart Schaefer; +Cc: zsh-workers
Bart Schaefer wrote on Fri, Jan 09, 2015 at 22:31:50 -0800:
> 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?
On FreeBSD systems, 'cat /path/to/dir' works (prints something and
returns 0). Therefore, even if the (symlink-to-)dir is not replaced by
a file, unexpected effects might be caused when some code attempts to
cat the tempfile (which is in fact a directory) — whether in order to
run it as shell commands or in order to use it as data.
Sorry, but I haven't audited the patch to see what exactly this could
cause in each instance. (No time.)
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: zsh/files module and insecure tempfile creation
2015-01-12 5:43 ` Daniel Shahaf
@ 2015-01-12 6:56 ` Bart Schaefer
0 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2015-01-12 6:56 UTC (permalink / raw)
To: zsh-workers
On Jan 12, 5:43am, Daniel Shahaf wrote:
} Subject: Re: zsh/files module and insecure tempfile creation
}
} Bart Schaefer wrote on Fri, Jan 09, 2015 at 22:31:50 -0800:
} > Next the attacker must be able to swap the directory or symlink with
} > a symlink to his own target file.
}
} On FreeBSD systems, 'cat /path/to/dir' works (prints something and
} returns 0).
That is also avoided, I think, by using zf_ln -fn to be sure the target
is removed and replaced by zsh's temp file. I think we're OK there after
this latest patch pass.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-12 6:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-10 6:31 zsh/files module and insecure tempfile creation Bart Schaefer
2015-01-10 19:58 ` Peter Stephenson
2015-01-11 2:04 ` Bart Schaefer
2015-01-12 5:43 ` Daniel Shahaf
2015-01-12 6:56 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).