Re: [PATCH v2 3/4] parse_commit(): handle broken whitespace-only timestamp

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> This probably doesn't matter in practice but we define our own
> isspace() that does not treat '\v' and '\f' as whitespace. However
> parse_timestamp() (which is just strtoumax()) uses the standard
> library's isspace() which does treat those characters as whitespace
> and is locale dependent. This means we can potentially stop at a
> character that parse_timestamp() treats as whitespace and if there are
> no digits after it we'll still walk past the end of the line. Using
> Rene's suggestion of testing the character with isdigit() would fix
> that. It would also avoid parsing negative timestamps as positive
> numbers and reject any timestamps that begin with a locale dependent
> digit.

A very interesting observation.  I wonder if a curious person can
craft a malformed timestamp with "hash-object --literally" to do
more than DoS themselves?

We are not going to put anything other than [ 0-9+-] after the '>'
we scan for, and making sure '>' is followed by SP and then [0-9]
would be sufficient to ensure strtoumax() to stop before the '\n'
but does not ensure that the "signal a bad timestamp with 0"
happens.  Perhaps that would be sufficient.  I dunno.

> I'm not familiar with this code, but would it be worth changing
> parse_timestamp() to stop parsing if it sees a newline?

Meaning replace or write our own strtoumax() equivalent?