threads / patch / 64686

patchreceive-pack: fix crash on out-of-namespace symref

Subject: [PATCH] receive-pack: fix crash on out-of-namespace symref

## tl;dr

7 messages between Dec 27, 2025 and Feb 22, 2026. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Troels Thomsen via GitGitGadget· Dec 27, 2025, 15:40 UTC · lore
From: Troels Thomsen <troels@thomsen.io>

`check_aliased_update_internal()` detects when a symbolic ref and its target are being updated in the same push. It does this by building a list of ref names without the optional namespace prefix. When a symbolic ref within a namespace points to a ref outside the namespace, `strip_namespace()` returns NULL which leads to a segfault.

A NULL check preventing this particular issue was repurposed in ded8393610. Rather than reintroducing it, we can instead build a list of fully qualified ref names. This prevents the crash, preserves the consistency check from da3efdb17b, and allows updates to all symbolic refs.

Signed-off-by: Troels Thomsen <troels@thomsen.io>
---
    receive-pack: fix crash on out-of-namespace symref
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2144%2Ftt%2Ffix-crash-on-out-of-namespace-symref-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2144/tt/fix-crash-on-out-of-namespace-symref-v1
Pull-Request: https://github.com/git/git/pull/2144
 builtin/receive-pack.c           |  9 ++++++---
 t/t5509-fetch-push-namespaces.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)
Show changes to 2 files +19 −3

builtin/receive-pack.c, t/t5509-fetch-push-namespaces.sh

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 9c49174616..a9e0568e94 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1685,7 +1685,6 @@ static void check_aliased_update_internal(struct command *cmd,
 		cmd->error_string = "broken symref";
 		return;
 	}
-	dst_name = strip_namespace(dst_name);
 
 	if (!(item = string_list_lookup(list, dst_name)))
 		return;
@@ -1730,10 +1729,13 @@ static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
 	struct string_list ref_list = STRING_LIST_INIT_NODUP;
+	struct strbuf ref_name = STRBUF_INIT;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		struct string_list_item *item =
-			string_list_append(&ref_list, cmd->ref_name);
+		struct string_list_item *item;
+		strbuf_reset(&ref_name);
+		strbuf_addf(&ref_name, "%s%s", get_git_namespace(), cmd->ref_name);
+		item = string_list_append(&ref_list, ref_name.buf);
 		item->util = (void *)cmd;
 	}
 	string_list_sort(&ref_list);
@@ -1743,6 +1745,7 @@ static void check_aliased_updates(struct command *commands)
 			check_aliased_update(cmd, &ref_list);
 	}
 
+	strbuf_release(&ref_name);
 	string_list_clear(&ref_list, 0);
 }
 
diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
index 095df1a753..3c333ccc5f 100755
--- a/t/t5509-fetch-push-namespaces.sh
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -175,4 +175,17 @@ test_expect_success 'denyCurrentBranch and unborn branch with ref namespace' '
 	)
 '
 
+test_expect_success 'pushing to symref pointing outside the namespace' '
+	(
+		cd pushee &&
+		git symbolic-ref refs/namespaces/namespace/refs/heads/main refs/heads/main &&
+		cd ../original &&
+		git push pushee-namespaced main &&
+		git ls-remote pushee-unnamespaced refs/heads/main >actual &&
+		printf "$commit1\trefs/heads/main\n" >expected &&
+		printf "$commit1\trefs/namespaces/namespace/refs/heads/main\n" >>expected &&
+		test_cmp expected actual
+	)
+'
+
 test_done

base-commit: 66ce5f8e8872f0183bb137911c52b07f1f242d13

-- 
gitgitgadget
Junio C Hamano· Dec 28, 2025, 14:57 UTC · re: Troels Thomsen via GitGitGadget · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

"Troels Thomsen via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 17 quoted lines
> From: Troels Thomsen <troels@thomsen.io>
>
> `check_aliased_update_internal()` detects when a symbolic ref and its
> target are being updated in the same push. It does this by building a
> list of ref names without the optional namespace prefix. When a symbolic
> ref within a namespace points to a ref outside the namespace,
> `strip_namespace()` returns NULL which leads to a segfault.
>
> A NULL check preventing this particular issue was repurposed in
> ded8393610. Rather than reintroducing it, we can instead build a list of
> fully qualified ref names. This prevents the crash, preserves the
> consistency check from da3efdb17b, and allows updates to all symbolic
> refs.
>
> Signed-off-by: Troels Thomsen <troels@thomsen.io>
> ---
>     receive-pack: fix crash on out-of-namespace symref

Fixing crash is certainly a good thing, but when the namespace is segregated and receive-pack wants to get updates only within the given namespace, would presence of such a cross namespace symref cause updates outside the namespace through the symref, defeating the point of setting up a namespace in the first place?

I am not objecting to the new behaviour, but am not sure if it is a sensible one. You _might_ be able to argue that an attempt to update underlying refs outside the namespace through such a symbolic ref should result in an error (i.e., a fix to the current crashing behaviour is to die in a controlled way).

Thoughts?
Troels Thomsen· Dec 28, 2025, 16:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

On Sun, Dec 28, 2025, at 15:57, Junio C Hamano wrote:
Show 13 quoted lines
> Fixing crash is certainly a good thing, but when the namespace is
> segregated and receive-pack wants to get updates only within the
> given namespace, would presence of such a cross namespace symref
> cause updates outside the namespace through the symref, defeating
> the point of setting up a namespace in the first place?
>
> I am not objecting to the new behaviour, but am not sure if it is a
> sensible one.  You _might_ be able to argue that an attempt to update
> underlying refs outside the namespace through such a symbolic ref
> should result in an error (i.e., a fix to the current crashing
> behaviour is to die in a controlled way).
>
> Thoughts?

I think it's important that the symbolic ref needs to be explicitly created on the receiving side.

An argument in favor of allowing updates is that you can still choose to reject them by implementing an update hook. Would the opposite be true? I explictly wanted to share a branch into a namespace and update it from there.

I suppose the behavior could be configurable. Given this bug has existed since 2016, I'm assuming namespaces and symbolic refs probably aren't used in combination frequently enough to justify this over using a hook.

-- 
Troels Thomsen
Junio C Hamano· Dec 30, 2025, 00:37 UTC · re: Troels Thomsen · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

"Troels Thomsen" <troels@thomsen.io> writes:
Show 18 quoted lines
> On Sun, Dec 28, 2025, at 15:57, Junio C Hamano wrote:
>
>> Fixing crash is certainly a good thing, but when the namespace is
>> segregated and receive-pack wants to get updates only within the
>> given namespace, would presence of such a cross namespace symref
>> cause updates outside the namespace through the symref, defeating
>> the point of setting up a namespace in the first place?
>>
>> I am not objecting to the new behaviour, but am not sure if it is a
>> sensible one.  You _might_ be able to argue that an attempt to update
>> underlying refs outside the namespace through such a symbolic ref
>> should result in an error (i.e., a fix to the current crashing
>> behaviour is to die in a controlled way).
>>
>> Thoughts?
>
> I think it's important that the symbolic ref needs to be explicitly
> created on the receiving side.

Yes, and that can cut both ways. In an ideal world without any end-users who make any mistakes, deliberate cross namespace symref may be a handy feature to break out of the namespace jail on purpose in a controlled way.

But if the symref was made to point across the namespace boundary by mistake, catching it as a misconfiguration may be a crucial chance the user has to prevent it from turning into a security incident. And that is why I asked.

Junio C Hamano· Feb 21, 2026, 17:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

Junio C Hamano <gitster@pobox.com> writes:
Show 30 quoted lines
> "Troels Thomsen" <troels@thomsen.io> writes:
>
>> On Sun, Dec 28, 2025, at 15:57, Junio C Hamano wrote:
>>
>>> Fixing crash is certainly a good thing, but when the namespace is
>>> segregated and receive-pack wants to get updates only within the
>>> given namespace, would presence of such a cross namespace symref
>>> cause updates outside the namespace through the symref, defeating
>>> the point of setting up a namespace in the first place?
>>>
>>> I am not objecting to the new behaviour, but am not sure if it is a
>>> sensible one.  You _might_ be able to argue that an attempt to update
>>> underlying refs outside the namespace through such a symbolic ref
>>> should result in an error (i.e., a fix to the current crashing
>>> behaviour is to die in a controlled way).
>>>
>>> Thoughts?
>>
>> I think it's important that the symbolic ref needs to be explicitly
>> created on the receiving side.
>
> Yes, and that can cut both ways.  In an ideal world without any
> end-users who make any mistakes, deliberate cross namespace symref
> may be a handy feature to break out of the namespace jail on purpose
> in a controlled way.
>
> But if the symref was made to point across the namespace boundary by
> mistake, catching it as a misconfiguration may be a crucial chance
> the user has to prevent it from turning into a security incident.
> And that is why I asked.

The review discussion thread ended here. I am dropping the topic out of my tree now, but I do not think it would be a bad idea to resurrect the topic that turns the uncontrolled segmentation fault into a controlled death that calls die("hey, what is that cross namespace link doing there?").

Thanks.
Troels Thomsen· Feb 22, 2026, 07:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

On Sat, Feb 21, 2026, at 18:00, Junio C Hamano wrote:
Show 40 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Troels Thomsen" <troels@thomsen.io> writes:
>>
>>> On Sun, Dec 28, 2025, at 15:57, Junio C Hamano wrote:
>>>
>>>> Fixing crash is certainly a good thing, but when the namespace is
>>>> segregated and receive-pack wants to get updates only within the
>>>> given namespace, would presence of such a cross namespace symref
>>>> cause updates outside the namespace through the symref, defeating
>>>> the point of setting up a namespace in the first place?
>>>>
>>>> I am not objecting to the new behaviour, but am not sure if it is a
>>>> sensible one.  You _might_ be able to argue that an attempt to update
>>>> underlying refs outside the namespace through such a symbolic ref
>>>> should result in an error (i.e., a fix to the current crashing
>>>> behaviour is to die in a controlled way).
>>>>
>>>> Thoughts?
>>>
>>> I think it's important that the symbolic ref needs to be explicitly
>>> created on the receiving side.
>>
>> Yes, and that can cut both ways.  In an ideal world without any
>> end-users who make any mistakes, deliberate cross namespace symref
>> may be a handy feature to break out of the namespace jail on purpose
>> in a controlled way.
>>
>> But if the symref was made to point across the namespace boundary by
>> mistake, catching it as a misconfiguration may be a crucial chance
>> the user has to prevent it from turning into a security incident.
>> And that is why I asked.
>
> The review discussion thread ended here.  I am dropping the topic
> out of my tree now, but I do not think it would be a bad idea to
> resurrect the topic that turns the uncontrolled segmentation fault
> into a controlled death that calls die("hey, what is that cross
> namespace link doing there?").
>
> Thanks.

Do you think your original concern could be addressed by adding a note to the security section of gitnamespaces?

It seems somewhat relevant that you're unlikely to create a symbolic ref within a namespace without first consulting the documentation to understand the ref format. Combined with the lack of interest in this thread, and the fact that no bug report was filed for years, I suspect this feature combination is rare. That's not a good reason for a bad default, but a symbolic ref can already point outside a namespace; you only can't update it.

If I fix it by rejecting updates as suggested, I still wouldn't be able to do what I wanted in the first place. Is there a better way to propose such a change?

In any case, thank you for your time.
-- 
Troels Thomsen
Junio C Hamano· Feb 22, 2026, 20:35 UTC · re: Troels Thomsen · lore

Re: [PATCH] receive-pack: fix crash on out-of-namespace symref

"Troels Thomsen" <troels@thomsen.io> writes:
> Do you think your original concern could be addressed by adding a note
> to the security section of gitnamespaces?

Not really. Nobody reads documentation, so it would be far more preferrable to make the default strict, with a documented way to optionally loosen, than the other way around.

← back to recent threads