Re: [PATCHv3] Updated patch series for providing mechanism to list available repositories

[Greg Brockman <gdb@MIT.EDU>]

Anders brings up a good point.

And note that as I alluded to before, there is another attack

$ echo 'DEFINE pager evilscript' > /tmp/.manpath
$ HOME=/tmp su git -m -c "git-receive-pack '--help'" (3)

which only requires being able to control HOME.

(Incidentally, I just noticed a segfault with

$ unset HOME
$ su git -m -c "git-receive-pack '~'"

that's probably worth fixing... if people don't think this is too
pedantic of a case to fix, I'll submit a patch for it in a later
series [I think the segfault comes from path.c:expand_user_path].)

Anyway, i'll revise my first patch to use HOME rather than getpw*.

Greg



On Wed, Jul 28, 2010 at 4:52 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Anders Kaseorg wrote:
>> On Wed, 2010-07-28 at 01:42 -0500, Jonathan Nieder wrote:
>
>>> (if you use getpwent instead of getenv to fetch $HOME).
>>
>> That seems like it could lead to problems with multiple users with the
>> same UID, and possibly also on Windows.  If it’s important to be
>> paranoid here, what about all the other places Git already uses
>> getenv("HOME"), including where it reads ~/.gitconfig?
>
> Thanks for a sanity check.  I do not see the multiple-user problem
> (git-shell is meant to be the login shell, no?) but I think you are
> right about using getwpwent instead of $HOME being a pointless
> precaution.  My confusion came from a misreading of how 'su' works.
>
> Here was my worry: that a user could do something like this:
>
>  $ mkdir /tmp/git-shell-commands
>  $ ln -s /bin/sh /tmp/git-shell-commands/sh
>  $ HOME=/tmp su git -m -c sh;           # (1)
>
> and get a shell with the privileges of the user with git-shell
> as login shell, which is exactly what a restricted shell like
> this should be preventing.
>
> Now if that is possible, what is to stop me from this?
>
>  $ PAGER=evilscript su git -m -c git-receive-pack --help; # (2)
>
> which became possible (modulo the su bit) as an unintended
> consequence when receive-pack became builtin.
>
> If I understand the manual correctly, then at least on some
> systems, luckily su protects correctly against such problems.
>
>        -m
>                Preserve the current environment.
>
>                If the target user has a restricted shell,
>                this option has no effect (unless su is
>                called by root).
>
> Is that behavior portable?  It certainly seems like the
> only sane way to behave.  It’s a moot question for the
> inclusion of this patch series: if we need to worry about
> (1), then it is still not a regression because (2) was possible
> already.
>
> The same discussion would seem to apply to ssh with
> PermitUserEnvironment enabled.
>