Re: [PATCH] object-name: reject too-deep recursive ancestor queries

From: Jeff King <peff@peff.net>
Date: Dec 6, 2023, 19:40 UTC
Message-ID: <20231206194035.GB103708@coredump.intra.peff.net>
In-Reply-To: <ZWB26TH0CFW1KC4L@tanuki>

On Fri, Nov 24, 2023 at 11:11:53AM +0100, Patrick Steinhardt wrote:

Show 8 quoted lines

> > When we get "HEAD~~~~~~~~~^2~~~~~~" from the user, do we somehow try
> > to create a file or a directory with that name and fail due to
> > ENAMETOOLONG?
> 
> Sorry, this was a typo on my part. I didn't mean "revision", I meant
> "reference" here. References are limited to at most 4kB on most
> platforms due to filesystem limitations, whereas revisions currently
> have no limits in place.

Even without filesystem limitations, references are effectively limited to 64kb due to the pkt-line format.

Revisions can be much longer than a reference, though. We accept "some_ref:some/path/in/tree", for instance[1]. I think you could argue that paths are likewise limited by the filesystem, though. Even on systems like Linux where paths can grow arbitrarily long (by descending and adding to the current directory), you're still limited in specifying a full pathname. And Git will always use the full path from the project root when creating worktree entries. Plus my recent tree-depth patches effectively limit us to 16MB in the default config.

So I think it might be reasonable to limit revision lengths just as a belt-and-suspenders against overflow attacks, etc. But I suspect that the limits we'd choose there might not match what we'd want for protection against stack exhaustion via recursion. E.g., I think 8k is probably the minimum I'd want for a revision ("my/4k/ref:my/4k/path"). If one "~" character can create an expensive recursion, that might be too much.

So we probably need something like Taylor's patch anyway (or to switch to an iterative algorithm, though that might be tricky because of the way we parse). I agree it needs to handle "^", though.

-Peff

[1] There are other more exotic revisions, too. The most arbitrary-sized
    that comes to mind is ":/some-string-to-match". I doubt anybody
    would be too mad if that were limited to 8k or even 4k, though.