# [PATCH] serve: reject valueless promisor-remote capability

3 messages from 2026-08-12 to 2026-08-13. Participants: Elijah Newren via GitGitGadget, Elijah Newren, Christian Couder.
Thread: https://gitlist.dev/t/66160

## Elijah Newren via GitGitGadget, 2026-08-12 06:39

Subject: [PATCH] serve: reject valueless promisor-remote capability
Message-ID: <pull.2199.git.1786516783909.gitgitgadget@gmail.com>

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

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

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    serve: reject valueless promisor-remote capability

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2199%2Fnewren%2Fpromisor-remote-require-argument-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2199/newren/promisor-remote-require-argument-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2199

 serve.c              |  3 +++
 t/t5701-git-serve.sh | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/serve.c b/serve.c
index 2b07d922b3..5a64344467 100644
--- a/serve.c
+++ b/serve.c
@@ -46,6 +46,9 @@ static int promisor_remote_advertise(struct repository *r,
 static void promisor_remote_receive(struct repository *r,
 				    const char *remotes)
 {
+	if (!remotes)
+		die("promisor-remote capability requires an argument");
+
 	mark_promisor_remotes_as_accepted(r, remotes);
 }
 
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index 9a575aa098..d888cc5c3c 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -71,6 +71,17 @@ test_expect_success 'request invalid capability' '
 	test_grep "unknown capability" err
 '
 
+test_expect_success 'promisor-remote capability requires an argument' '
+	test-tool pkt-line pack >in <<-EOF &&
+	command=ls-refs
+	object-format=$(test_oid algo)
+	promisor-remote
+	0000
+	EOF
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
+	test_grep "promisor-remote capability requires an argument" err
+'
+
 test_expect_success 'request with no command' '
 	test-tool pkt-line pack >in <<-EOF &&
 	agent=git/test

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 
gitgitgadget

```

## Elijah Newren, 2026-08-12 21:27

Subject: Re: [PATCH] serve: reject valueless promisor-remote capability
Message-ID: <CABPp-BGKfojr8wbQdkSegm_bL5r0t51_+qc7k74JMoKp4MDw3g@mail.gmail.com>
In-Reply-To: <pull.2199.git.1786516783909.gitgitgadget@gmail.com>

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

```

## Christian Couder, 2026-08-13 15:27

Subject: Re: [PATCH] serve: reject valueless promisor-remote capability
Message-ID: <CAP8UFD0+iXC3VxWmuuuB7La-pP6hdz58tr6vaEJSKpXJ_4ZH2w@mail.gmail.com>
In-Reply-To: <pull.2199.git.1786516783909.gitgitgadget@gmail.com>

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

```