[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Reminder: $EDITOR with spaces?

From: Mark Benedetto King <bking_at_answerfriend.com>
Date: 2002-04-29 20:20:02 CEST

On Mon, Apr 29, 2002 at 01:13:54PM +0200, Marcus Comstedt wrote:
>
> Mark Benedetto King <bking@answerfriend.com> writes:
>
> > Historically, many vulnerabilities have resulted from insufficient
> > input validation around system() calls. Considering the complexity
> > of /bin/sh, this is not suprising.
>
> Certainly. However, they are not really relevant here.
>
>
> > This will seem a little contrived, but it is an example:
> >
> > Let's say an administrator wants to build s setuid-svn executable
> > that is run as follows:
> >
> > mycommit foo
> >
> > and will eventuall call, as uid=svn:
> >
> > EDITOR="/path/to/editor foo" svn commit
> >
> > Let's not try to understand *why* the administrator might
> > want to do this, just that it is possible. :-)
>
> Here, the problem is not with svn using system(). Instead, the
> problem is with the setuid program mycommit building a command line
> from user input without proper quoting. A big no-no in any setuid

The problem is not with mycommit or with system(). It's with
the way they're (hypothetically) interacting. mycommit is
expecting execl() semantics and system() doesn't give those
semantics. This has bitten many programmers many times.

> program. You can not expect svn to guard you against vulnerabilities
> in _other_ programs. That's a "can't win, don't try" type scenario.

It might be a "can't win, don't use system()" type scenario,
though. Look at gets(). It's so evil, that it has been
thoroughly deprecrated. Is gets() secure? Sure, if it never
runs with elevated privileges or if the stdin has been sufficient
constrained so as to ensure that lines will always be short enough.
Most people have decided, at least for gets(), that the answer
is "don't use gets()".

>
>
> > So, that means before we call system, either we need
> > to escape all semicolons (and pipes and backticks and ...)
>
> No. That would defeat the main purpose of using system().

I agree. And it would lose if system() used a /bin/sh with
different semantics than those expected. *that* is a "can't
win, don't try" problem. :-) I was presenting it as the
"naive approach to be shot down later in my argument".

> > 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."
>
> Somebody who needs that warning probably wasn't fit to build setuid
> applications in the first place... Anyway, if you do a setuid app you

You're right, but they do. That's why, as has been pointed out,
perl has taint.

> need to reset $EDITOR regardless. It doesn't take any "craftily
> constructed $EDITOR variables" to exploit a setuid program that calls
> $EDITOR. You can even leave it set to "emacs", just do M-x shell when

Sure it does, if it the user expects execl() semantics
and gets system() semantics.

Considering that the huge install-base of programs that use
$EDITOR seem to all give system() semantics, it shouldn't
be too much of a surprise. However, that discrepancy
is the source of the "security problems with system".

--ben

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 29 20:23:59 2002

This is an archived mail posted to the Subversion Dev mailing list.