Re: [PATCH v10 8/8] cat-file: add remote-object-info to batch-command

>
> On Mon, Jan 13, 2025 at 09:15:00PM -0500, Eric Ju wrote:
>
> > +static void parse_cmd_remote_object_info(struct batch_options *opt,
> > +                                      const char *line, struct strbuf *output,
> > +                                      struct expand_data *data)
> > +{
> > +     int count;
> > +     const char **argv;
> > +
> > +     char *line_to_split = xstrdup_or_null(line);
> > +     count = split_cmdline(line_to_split, &argv);
> > +     if (get_remote_info(opt, count, argv))
> > +             goto cleanup;
>
> Coverity complains that split_cmdline() can return a negative value when
> the input is malformed, which we then feed to get_remote_info(). If I
> understand correctly (from my very brief glance at the series), that
> string would be under the control of the untrusted client?
>
> I _think_ an attacker can't do anything too bad here, since
> get_remote_info() also takes a signed int, and so iterating from 0 will
> just find no entries. But probably we should explicitly check for error
> and bail.
>

> While just looking at this code from a security perspective, two other
> things occur to me:
>
>   1. Calling xstrdup_or_null() implies that "line" may be NULL, which
>      would make "line_to_split" also NULL. But I think split_cmdline()
>      would segfault in that case. Should it just be xstrdup()?
>

>   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.
>