Subject: Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing

From: Michael Haggerty <[email-masked]>
Date: Wed, 18 Mar 2015 11:20:09 +0100

On 03/18/2015 11:03 AM, Jeff King wrote:
> 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
[email-masked]