Re: [PATCH 8/9] fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 12, 2025, 11:25 UTC
- Message-ID
- <aRRux2uBfORc214r@pks.im>
- In-Reply-To
- <20251112081040.GH979063@coredump.intra.peff.net>
On Wed, Nov 12, 2025 at 03:10:40AM -0500, Jeff King wrote:
Show 18 quoted lines
> In fsck_ident(), we parse the timestamp with parse_timestamp(), which is > really an alias for strtoumax(). But since our buffer may not be > NUL-terminated, this can trigger a complaint from ASan's > strict_string_checks mode. This is a false positive, since we know that > the buffer contains a trailing newline (which we checked earlier in the > function), and that strtoumax() would stop there. > > But it is worth working around ASan's complaint. One is because that > will let us turn on strict_string_checks by default, which has helped > catch other real problems. And two is that the safety of the current > code is very hard to reason about (it subtly depends on distant code > which could change). > > One option here is to just parse the number left-to-right ourselves. But > we care about the size of a timestamp_t and detecting overflow, since > that's part of the point of these checks. And doing that correctly is > tricky. So we'll instead just pull the digits into a separate, > NUL-terminated buffer, and use that to call parse_timestamp().
So this is another site that would benefit from having something like `git_parse_int()` with an extra parameter indicating the number of bytes available for parsing (and a way to disable unit factors).
Patrick