serve: reject valueless promisor-remote capability

[newren]

cc: Elijah Newren newren@gmail.com
cc: Christian Couder christian.couder@gmail.com

[newren]

/submit

[gitgitgadget]

Submitted as pull.2199.git.1786516783909.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2199/newren/promisor-remote-require-argument-v1

To fetch this version to local tag pr-2199/newren/promisor-remote-require-argument-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2199/newren/promisor-remote-require-argument-v1

[gitgitgadget]

Elijah Newren wrote on the Git mailing list (how to reply to this email):

On Tue, Aug 11, 2026 at 11:39 PM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> d460267613da (Add 'promisor-remote' capability to protocol v2,
> 2025-02-18) added a receive callback which passes the capability value
> directly to mark_promisor_remotes_as_accepted(). However, a client can
> send the capability name without an '=' or value, in which case
> get_capability() supplies NULL and strbuf_split_str() dereferences it.

Oops, I previously forgot to CC Christian as the author of
d460267613da.  Doing that now.

[gitgitgadget]

User Elijah Newren <newren@gmail.com> has been added to the cc: list.

[gitgitgadget]

This branch is now known as en/serve-promisor-remote-fix.

[gitgitgadget]

This patch series was integrated into seen via git@9737e16.

[gitgitgadget]

There was a status update in the "New Topics" section about the branch en/serve-promisor-remote-fix on the Git mailing list:

A client requesting the promisor-remote capability without a value
caused a null pointer dereference, which has been corrected by
rejecting a request without an argument.

Needs review.
source: <pull.2199.git.1786516783909.gitgitgadget@gmail.com>

[gitgitgadget]

Christian Couder wrote on the Git mailing list (how to reply to this email):

On Wed, Aug 12, 2026 at 8:42 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> d460267613da (Add 'promisor-remote' capability to protocol v2,
> 2025-02-18) added a receive callback which passes the capability value
> directly to mark_promisor_remotes_as_accepted(). However, a client can
> send the capability name without an '=' or value, in which case
> get_capability() supplies NULL and strbuf_split_str() dereferences it.

Yeah, the original code you mention used strbuf_split_str(), but since
68a746e9a8 (promisor-remote: use string_list_split() in
mark_remotes_as_accepted(), 2025-09-08), string_list_split() is used
instead. Anyway string_list_split() also crashes when a NULL is passed
as its `const char *string` argument.

> Reject the missing argument before parsing it, and add a test covering
> this case.

Yeah, the fix and its test look right to me. Thanks.

[gitgitgadget]

User Christian Couder <christian.couder@gmail.com> has been added to the cc: list.

[gitgitgadget]

There was a status update in the "Cooking" section about the branch en/serve-promisor-remote-fix on the Git mailing list:

A client requesting the promisor-remote capability without a value
caused a null pointer dereference, which has been corrected by
rejecting a request without an argument.

Will merge to 'next'?
cf. <CAP8UFD0+iXC3VxWmuuuB7La-pP6hdz58tr6vaEJSKpXJ_4ZH2w@mail.gmail.com>
source: <pull.2199.git.1786516783909.gitgitgadget@gmail.com>