>
> So, that means before we call system, either we need
> to escape all semicolons (and pipes and backticks and ...)
> or we need to warn the user "$EDITOR is passed verbatim
> to system; you are probably *not* smart enough to protect
> yourself from craftily constructed $EDITOR variables, so
> we recommend that you don't build setuid applications that
> can trigger $EDITOR."
>
>
> --ben
>
Good point, Ben. We can (should? must?) do something like Perl taint
checking
does: refuse to invoke an $EDITOR if we're running setuid. Even if we
did escape all {semicolon,pipe,tick,etc.}, setuid binaries could still
be exploited with:
export EDITOR="my-script-to-do-something-nasty.sh"
setuidcommit any.file
When not running setuid, I still think system() is the best way to go
because it provides the same semantics for $EDITOR as other programs
which share the *very same variable*. If we want to do our own parsing
for safety/cross-platformness/etc to the $SVN_EDITOR variable, that's
fine, because that's not breaking compatibility with other programs.
I haven't come up with a good example, but here's a contrived example of
how doing our own parsing (different from the shell) could be bad:
export EDITOR="vim -n"
cvs commit somefile.x
svn commit somefile.y
If I have a "vim -n" executable somewhere in my path, then it will be
called by svn but not cvs. Not what I would expect as a longtime CVS
user. (See, I told you it was contrived.)
Anyway, I say don't muck with the standard meaning of $EDITOR which
seems to be "pass it to system". (Though I don't know anything about
how this applies on Windows.)
And never invoke system() on any tainted (user specified) data when
running setuid. (Or open output files whose names are derived from
tainted data. Or system()/exec() any program when $PATH hasn't been set
to an untainted value. Or run any world-writable executable. Or any
executable in a world writable directory. Or... there are probably a few
dozen things to watch out for when running setuid.) See the perlsec man
page for more.
Being safe sounds like a lot of work. Instead, maybe we should bail out
if running setuid? Or loudly warn a whole lot? Or should we just put a
warning somewhere in small print at the bottom of INSTALL?
-David Mankin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 29 09:26:51 2002