Re: [PATCH] archive: error instead of triggering a segfault in `git archive --remote=""`

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 10, 2025, 15:59 UTC
Message-ID: <xmqq7c4wanb8.fsf@gitster.g>
In-Reply-To: <Z86WtQ-3Rk9OSmn_@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 7 quoted lines

>> +	if (remote && !remote[0]) {
>
> Okay, so this triggers in case the user passes "--remote ''"?
> I see that we ultimately pass the string to `remote_get()`, so does that
> function segfault? If so, can other callers of that function segfault in
> a similar way? In that case, we should probably address the issue deeper
> down in the call stack.

A good thing to point out. If remote_get() segfaults, that is a grave bug. If remote_get() returns a NULL for such a non-existent remote, the code should be able to cope with it, or you found a bug.

In short, I agree with you that this may merely be sweeping a problem under a rug, not addressing a real problem.

run_remote_archiver() seems to run remote_get() and use the returned value (which could be NULL, if you named a remote nickname that you do not even have) without validating when it calls transport_get(), so that is probably where the problem lies. If I were writing this code path, I would probably make run_remote_archiver() take a pointer to an instance of "struct remote", moving the call to remote_get() to the caller's side, and deal with an error inside cmd_archive().

Thanks.