On Sat, Feb 10, 2007 at 09:34:38PM +0100, Johannes Schindelin wrote:
quoted text
> > +		if (alias_string[0] == '!') {
> > +			trace_printf("trace: alias to shell cmd: %s => %s\n",
> > +				     alias_command, alias_string+1);
> 
> Here, you add 1 to alias string (though I would put spaces around the 
> plus, but that's really a nit).

That's not how I code but it does seem to be the prevailing git coding
style, so I'll change it.

quoted text
> > +			die("Failed to run '%s' when expanding alias '%s'\n", 
> > +			    alias_string, alias_command);
> 
> So, shouldn't you here, too?

Yes, that makes the error message look a bit nicer.  I'll respin the
patch.

quoted text
> It made me feel a little uneasy that we can execute _any_ command now, but 
> I can only find one way to exploit this, when an attacker does not have 
> shell access anyway: git-shell.

... and git-shell only allows git-receive-pack and git-upload-pack to
be called, with a single argument, and aliases aren't allowed to
override commands.  So we're safe here, I think.

						- Ted
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html