[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 19:57:30 CEST

On Mon, Apr 29, 2002 at 12:25:12PM -0500, Karl Fogel wrote:
> > So, let's say the user runs
> >
> > mycommit "foo; cp /bin/sh /tmp/; chmod 4777 /tmp/sh"
>
> Invoking an $EDITOR by *any* means is the same as invoking arbitrary
> programs (for example, if it's Emacs, then the user can just start up
> a shell inside the editor, no need to get fancy). Bottom line: if you
> can run `svn' as user jrandom, then you can run any program as user
> jrandom. If someone has installed a setuid svn client binary, they
> deserve what they get -- the safe thing to do is make sure only
> jrandom can run svn as jrandom.

The case that I'm pointing out is where a insufficiently paranoid
operator has done enough input validation to verify the the first
white-space-separated component of $EDITOR is what he wants. The
case in which the semantics of execl() are different from system().

This has happened again and again. It should, however be sufficient
to warn in the man page that $EDITOR will be passed to system(),
rather than execl().

I guess.

>
> Remember, this isn't just about $EDITOR. It also holds when we
> provide a mechanism for invoking a user-specified diff or merge
> program; and inevitably there will be other circumstances where svn
> invokes subprocesses. We need to put a warning in the documentation,
> but otherwise let people shoot themselves in the foot if they choose.
>

I agree.

> So far, the security implications of system() don't seem any worse
> than using APR/execl; system() is effectively as portable (it's ANSI
> C, and the rest of the portability issues boil down to the argument
> passed); and using system() will get us some expected behaviors that
> we're not getting with APR.
>

No, you're wrong. The semantics are different. If the administrator
expects execl() semantics they will be very suprised (and perhaps
compromised) when they get system() semantics.

> +1 on using system(). I'm probably going to make this change unless
> someone (Daniel?) beats me to it... :-)

All that said, +1 on using system() from me, too, because
it *is* a lot more powerful than execl(), and because it's
what people expect from $EDITOR.

--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:01:41 2002

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