Subject: [PATCH 0/4] more robust functions for parsing int from buf

From: Jeff King <[email-masked]>
Date: Sun, 30 Nov 2025 08:13:51 -0500

On Wed, Nov 26, 2025 at 09:22:38AM -0800, Junio C Hamano wrote:

> Jeff King <[email-masked]> writes:
> 
> > Hmm, I thought both of those things were reasonably clever. The other
> > obvious way to do it, AFAICT, is to used checked-operation intrinsics or
> > add unsigned_add_overflows() before every operation.
> 
> Yup, but the thing is, I didn't want something "clever".  I prefer
> "clean and obvious" if we add extra code for safety.

Yeah, that's fair. It turns out that one half of that is easy: checking
for overflow as we compute the number). And one half is hard. If you
don't assume a twos-complement style range where the "min = -max - 1",
then you are stuck using INT_MIN. Which is OK for "int", but not for
arbitrary types. We already make the same assumption in git_parse_int(),
etc.

So I went with that approach here, but it is at least documented
clearly.

> > It looks like you merged what I had into 'next'. Where do you want to go
> > from there? I am mostly content to let it be, but we can also try to
> > replace with something like your version.
> 
> That is my preference.  While the topic is still in 'next', or after
> the topic graduates to 'master'.  Either is fine.  And it is fine if
> such an update did not come, too.  After all, this is to deal with
> contents in a locally generated file (.git/index), so a maliciously
> corrupt string that lack the expected whitespace character after the
> digit string is a sign that you are trying to burn yourself and you
> have only yourself to blame, isn't it?  An attacker that can put
> garbage in your .git/index has better ways to fool you by updating
> your .git/config file that sits next to it.  Or teach the sanitizer
> that this code path is already OK somehow?

Yeah, I agree the stakes are low here. Though they were somewhat low to
begin with for the same reason! But I was grossed out enough by the
whole thing that I tried to put together a decent helper for parsing
integers from buffers, and converted both sites here.

I suspect it could be used in other places, too, but I didn't convert
any.

> > Or even, I guess, work on a
> > global strntoi() that could be used everywhere, if we think it is robust
> > enough. (Though technically that name is reserved by the standard, which
> > is a shame, because that is really what this thing is).
> 
> Well, we already use plenty of names beginning with 'str' followed
> by a lowercase letter, like strbuf_foo() and string_list_init().

In the end it was sufficiently different from strtoi() that I decided
not to use that name. It was but one of many bike-sheddable decisions,
which I tried to document. So I guess let the flaming commence. ;)

This is built on top of jk/asan-bonanza.

  [1/4]: parse: prefer bool to int for boolean returns
  [2/4]: parse: add functions for parsing from non-string buffers
  [3/4]: cache-tree: use parse_int_from_buf()
  [4/4]: fsck: use parse_unsigned_from_buf() for parsing timestamp

 Makefile                   |   1 +
 cache-tree.c               |  28 ++-----
 compat/posix.h             |   2 +
 fsck.c                     |  20 +----
 parse.c                    | 162 +++++++++++++++++++++++++++++--------
 parse.h                    |  31 +++++--
 t/meson.build              |   1 +
 t/unit-tests/u-parse-int.c |  98 ++++++++++++++++++++++
 8 files changed, 263 insertions(+), 80 deletions(-)
 create mode 100644 t/unit-tests/u-parse-int.c

-Peff