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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

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