Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

[Jeff King <peff@peff.net>]

On Thu, Jul 07, 2016 at 02:41:37PM -0700, Stefan Beller wrote:

> >> +     /* NEEDSWORK: expose the limitations to be configurable. */
> >> +     int max_options = 32;
> >> +
> >> +     /*
> >> +      * NEEDSWORK: expose the limitations to be configurable;
> >> +      * Once the limit can be lifted, include a way for payloads
> >> +      * larger than one pkt, e.g allow a payload of up to
> >> +      * LARGE_PACKET_MAX - 1 only, and reserve the last byte
> >> +      * to indicate whether the next pkt continues with this
> >> +      * push option.
> >> +      */
> >> +     int max_size = 1024;
> >
> > Good NEEDSWORK comments; perhaps also hint that the configuration
> > must not come from the repository level configuration file (i.e.
> > Peff's "scoped configuration" from jk/upload-pack-hook topic)?
> 
> Ok, I reviewed that series. It is unclear to me how the attack would
> actually look like in that case.
> 
> In 20b20a22f8f Jeff writes:
> > Because we promise that
> > upload-pack is safe to run in an untrusted repository, we
> > cannot execute arbitrary code or commands found in the
> > repository (neither in hooks/, nor in the config).
> 
> I agree on this for all content that can be modified by the user
> (e.g. files in the work tree such as .gitmodules), but the .git/config
> file cannot be changed remotely. So I wonder how an attack would
> look like for a hosting provider or anyone else?
> We still rely on a sane system and trust /etc/gitconfig
> so we do trust the host/admin but not the user?

The problem is for hosting sites which serve repositories via git-daemon
from untrusted users who have real shell accounts (e.g., you set up
git-daemon to run as the "daemon" user serving repositories out of
people's home directories; you don't want users to escalate their shell
access into running arbitrary code as "daemon").

But I don't think that case applies here. That is about running
upload-pack on an untrusted repository, but your changes here are part
of receive-pack. In such a scenario, users should be pushing as
themselves via ssh. And if they are not (e.g., the admin set up
push-over-smart-http centrally), they are already screwed, as a
malicious user could just set up a pre-receive hook.

IOW, we promise only that upload-pack is safe to run an untrusted repo,
but not receive-pack.

-Peff