On Mon, Feb 24, 2025 at 6:45 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Fri, Feb 21, 2025 at 10:34:44AM -0500, Peijian Ju wrote:
>
> > Thank you. Revised to use xstrdup() in v11.
> >
> > > 2. Are there any bounds on the size of "line"? E.g., is it coming in
> > > as a single pkt, or can it be arbitrarily large if an attacker
> > > wants (it looks like maybe the latter, since it comes from a strbuf
> > > in batch_objects_command(), but I didn't look at how network data
> > > gets passed in to that). At any rate, I think we ran into problems
> > > before with split_cmdline() and integer overflow, since it returns
> > > an int (CVE-2022-39260). I thought we fixed it by rejecting long
> > > lines in git-shell, but it looks like we also hardened
> > > split_cmdline() in 0ca6ead81e (alias.c: reject too-long cmdline
> > > strings in split_cmdline(), 2022-09-28).
> > >
> > > So we are maybe OK, but I wonder if we should punt on absurd lines.
> > > Related, can an attacker just flood input into that strbuf, making
> > > it grow forever and waste memory? That's just a simple resource
> > > attack, but we have tried to avoid those elsewhere in upload-pack,
> > > etc.
> > >
> >
> > Thank you. Adding a check in v11 for the length of `lines`. Please let
> > me know if something like this makes sense:
> >
> > if (strlen(line) >= INT_MAX) {
> > die(_("remote-object-info command input overflow"));
> > }
>
> I took a look at what you ended up with in v11, and...I think I totally
> misunderstood what was going on in your series, or when this code would
> be run.
>
> I had thought the cat-file here was running on the server side, and that
> we needed to protect ourselves against malicious clients. But your new
> parse_cmd_remote_object_info() is purely a client-side function that
> will then access the server behind the scenes. And its input will be
> coming from the stdin of cat-file locally.
>
> So I'm not sure that we need to protect it unless we think there's some
> way that an attacker can automatically trigger arbitrary
> remote-object-info requests.
>
Thank you. Yes, remote-object-info is purely a client-side command.If
an attacker is able to automatically
trigger arbitrary remote-object-info requests, it likely means they
already have control over that system.
