Re: [PATCH v4 4/6] list-objects: filter objects in traverse_commit_list

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Nov 17, 2017, 15:42 UTC
Message-ID: <6f433987-f91b-d5b5-242e-3a241b7442c7@jeffhostetler.com>
In-Reply-To: <xmqqh8tttzwq.fsf@gitster.mtv.corp.google.com>

On 11/16/2017 9:14 PM, Junio C Hamano wrote:

Show 16 quoted lines

> Jeff King <peff@peff.net> writes:
> 
>> Those encodings don't necessarily need to be the same, because they're
>> about transport. Inside each process we'd have the raw bytes, and encode
>> them as appropriate to whatever sub-program we're going to pass to (or
>> not at all if we skip the shell for sub-processes, which is usually a
>> good idea).
> 
> Yes, I share the same feeling.  It does not help that the series
> defines its own notion of arg_needs_armor() and uses it to set a
> field called requires_armor that is not yet used, the definition of
> "armor"ing being each byte getting encoded as two hexadecimal digits
> without any sign (which makes me wonder what a receiver of
> "deadbeef" would do---did it receive an armored string or a plain
> one???).  I do not understand why these strings are not passed as
> opaque sequences of bytes and instead converted at this low a layer.

I'm probably being too paranoid. My fear is that a client could pass an expression to clone/fetch/fetch-pack that would be sent to the server and evaluated by the interface between upload-pack and pack-objects. I'm not worried about the pack-protocol transport. I'm mainly concerned in how upload-pack passes that *client-expression* to pack-objects and are there ways for that to go south on the server with a carefully crafted expression.

Even if we assume that upload-pack on the server directly invokes pack-objects (rather than a shell), there still might be issues. For platforms like Linux which have a native execve() and can pass args in an argv (and which the sub-process also receives in an argv in their main()), my paranoia is probably overkill.

But on Windows, where the native interface takes a command-line string rather than an argv, I was concerned. Yes, there is code in compat/mingw.c to quote args when building a command line from an argv (and I'm *not* saying there are bugs in that), but again maybe I am being paranoid.

I'll take another look and the existing quoting mechanisms and re-eval.

Thanks, Jeff