[darcs-devel] darcs patch: added postget to prefs (and 2 more)

[David Roundy]

On Wed, Aug 03, 2005 at 11:55:29PM -0700, Jason Dagit wrote:
> On Aug 3, 2005, at 5:45 AM, David Roundy wrote:
> >ALL --post-hook "echo darcs $COMMAND @FLAGS @ARGS >> /path/to/log"
>
> You make a convincing argument for using env variables, but, if I  
> understand the env variables correctly they allow for some security  
> problems:

It's not quite *my* argument, since I got it from someone else who was
trying to convince me of the same thing, a while back...  :)

> 1) in your example, what if $COMMAND is expanded in a way that ends  
> the call to echo and then injects something malicious?

I know there's a way to empty out the environment before calling a
subprocess, and we should do that (so we're safe from attacks that define
an environment variable before calling darcs).

$COMMAND here is the darcs command, so that's "safe", but a bad person
could attack you by somehow convincing you to call

darcs whatsnew 'scary_filename;rm -rf ~/'

> 2) other users on a unix system can see other user's command lines.   
> so we'd be asking people to only put non-confidential information in  
> patches.

But users can't see the environments of other users' processes can they? I
don't see how passing information as environment variables could be a
security leak.  True, my "echo" example would be a potential security leak,
but that's the user's choice.

If the users don't want to make visible these details, they could always
write a script to do the work instead of using a simple one-liner.

> I think if we combine our ideas we'd be okay.  Have darcs always  
> write the details to files (one file for each type of detail), and  
> then the names of the files are stored in the env variables you  
> mention.  This way, no confidential information is in the env, and  
> since darcs would have control over the names of the files there is  
> no security risk for using them on the command line.  Your example  
> would then be:
> 
> ALL --post-hook "echo -n \"darcs \"; cat $COMMAND @FLAGS @ARGS >> / 
> path/to/log"

This seems cumbersome to me.  But he who implements gets to choose the
implementation...

> Speaking of security, the posthook for apply opens a can of worms if  
> people want to place the posthook script under darcs's control.  I  
> don't know how often people will realize it's a problem, or how we  
> could discourage it.  For one thing, we can point it out in the  
> manual and that's probably a good place to raise awareness that if  
> your posthook script is versioned then someone could send in a patch  
> that writes over it before it runs.

It's worth mentioning, but I wouldn't worry about it too much.  Any person
who sets it up for any program to be run automatically in response to
someone else's input needs to be aware of the security risks.  Even if the
hook script isn't under darcs control, it's still needs to be carefully
audited (and probably won't be) if you're going to run apply automatically
based on untrusted input.
-- 
David Roundy
http://www.darcs.net