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

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

[Troels Thomsen via GitGitGadget]

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(-)

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

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

[Junio C Hamano]

"Troels Thomsen via GitGitGadget" <gitgitgadget@gmail.com> writes:

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

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

[Troels Thomsen]

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.

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

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

[Junio C Hamano]

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