Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Mar 18, 2015, 10:20 UTC
- Message-ID
- <55095159.40106@alum.mit.edu>
- In-Reply-To
- <20150318100338.GA26998@peff.net>
On 03/18/2015 11:03 AM, Jeff King wrote:
Show 24 quoted lines
> On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote:
>
>> But in case you have some reason that you want upload-pack.c to be
>> converted right away, I just pushed that change (plus some related
>> cleanups) to my GitHub repo [1]. The branch depends only on the first
>> patch of the "numparse" patch series.
>>
>> By the way, some other packet line parsing code in that file doesn't
>> verify that there are no trailing characters on the lines that they
>> process. That might be another thing that should be tightened up.
>
> Do you mean that upload-pack gets a pkt-line of length N that contains a
> line of length M, and then doesn't check that M==N? We use the space
> between M and N for passing capabilities and other metadata around.
>
> Or do you mean that we see lines like:
>
> want [0-9a-f]{40} ...\n
>
> and do not bother looking at the "..." that comes after the data we
> expect? That I can believe, and I don't think it would hurt to tighten
> up (we shouldn't need it for extensibility, as anybody trying to stick
> extra data there should do so only after using a capability flag earlier
> in the protocol).The latter. For example here [1], the "have" command and its SHA-1 are read from the line, but I don't see a check that there are no characters after the SHA-1. The same here [2].
Michael
[1] https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L404-L429 [2] https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L550-L565
-- Michael Haggerty mhagger@alum.mit.edu