Checks added for CVE-2018-11235 too restrictive?

[Mike Hommey <mh@glandium.org>]

Hi,

(Background) I'm the author of a git remote-helper that can talk
directly to mercurial servers, known as git-cinnabar. One of the design
decisions was to use git objects to store all the metadata necessary to
reconstruct mercurial changesets, manifests and files, and one special
thing that's done is that git trees are (ab)used to store mercurial
sha1s in commit references. This design decision was made so that the
metadata could possibly be exchanged, allowing to jumpstart clone of
large repositories faster than by converting from scratch from
mercurial.

I had a first shot at that a few months ago, but the format of the
metadata branch made it impossible to push to github, hitting its push
size limit. With some pre-processing work, I was able to push the data
to github, with the intent to come back with a new metadata branch
format that would make the push directly possible.

Fast forward to this week, where I was trying to upload a new metadata
branch, and failed in a rather interesting way: multiple lines looking
like:

remote: error: object 81eae74b046d284c47e788143bbbcc681cb53418: gitmodulesMissing: unable to read .gitmodules blob

which, apart from the fact that they have some formatting issue, appear
to be new from the fixes for CVE-2018-11235.

I can see what those fixes are trying to prevent, but they seem to be
overly restrictive, at least in the context of transfer.fsckObjects.

The core problem is that the mercurial repository has some .gitmodules
files in some subdirectories. As a consequence, the git objects storing
the info for those mercurial files contain trees with .gitmodules files
with a commit ref, and that's what the remote is complaining about.

(Surpringly, it doesn't hit the "non-blob found at .gitmodules" case
first, which is weird).

A small testcase to reproduce looks like this:

$ git init bar; cd bar
$ git fast-import <<EOF
commit refs/heads/bar
committer Bar <bar@bar> 0 +0000
data 0
                                                                 
M 160000 81eae74b046d284c47e788143bbbcc681cb53418 bar/.gitmodules
                                                                 
EOF
$ git init ../qux
$ git -C ../qux config receive.fsckObjects true
$ git push -q ../qux bar
remote: error: object 81eae74b046d284c47e788143bbbcc681cb53418: gitmodulesMissing: unable to read .gitmodules blob
remote: fatal: fsck error in pack objects
error: remote unpack failed: unpack-objects abnormal exit
To ../qux
 ! [remote rejected] bar -> bar (unpacker error)
error: failed to push some refs to '../qux'

Would it be reasonable to make the transfer.fsckObject checks ignore
non-blob .gitmodules?

Cheers,

Mike