On Thu, Feb 22, 2018 at 04:44:02PM -0500, Jeff King wrote:
> But I don't think it _is_ an accident waiting to happen for the rest of
> the commands. upload-pack is special. The point is that people may touch
> git.c thinking they are adding a nice new feature (like pager config, or
> aliases, or default options, or whatever). And it _would_ be a nice new
> feature for most commands, but not for upload-pack, because its
> requirements are different.
>
> So thinking about security in the git wrapper is just a burden for those
> other commands.
All of that said, I think the current code is quite dangerous already,
and maybe even broken. upload-pack may run sub-commands like rev-list
or pack-objects, which are themselves builtins.
For example:
git init --bare evil.git
git -C evil.git --work-tree=. commit --allow-empty -m foo
git -C evil.git config pager.pack-objects 'echo >&2 oops'
git clone --no-local evil.git victim
That doesn't _quite_ work, because we route pack-objects' stderr into a
pipe, which suppresses the pager. But we don't for rev-list, which we
call when checking reachability. It's a bit tricky to get a client to
trigger those for a vanilla fetch, though. Here's the best I could come
up with:
git init --bare evil.git
git -C evil.git --work-tree=. commit --allow-empty -m one
git -C evil.git config pager.rev-list 'echo >&2 oops'
git init super
(
cd super
# obviously use host:path if you're attacking somebody over ssh
git submodule add ../evil.git evil
git commit -am 'add evil submodule'
)
git -C evil.git config uploadpack.allowReachableSHA1InWant true
git -C evil.git update-ref -d refs/heads/master
git clone --recurse-submodules super victim
I couldn't quite get it to work, but I think it's because I'm doing
something wrong with the submodules. But I also think this attack would
_have_ to be done over ssh, because on a local system the submodule
clone would a hard-link rather than a real fetch.
-Peff
