packfile.c: prevent overflow in `nth_packed_object_id()`
commitde41d03e1c7ab73174716c99b8eaf7ff5884d6bb
authorTaylor Blau <me@ttaylorr.com>
Wed, 12 Jul 2023 23:37:27 +0000 (12 19:37 -0400)
committerJunio C Hamano <gitster@pobox.com>
Thu, 13 Jul 2023 04:44:59 +0000 (12 21:44 -0700)
tree5ef79cbc92e4950dc2f013a17fb84ca655e2986b
parentfb7d80edcae482f4fa5d4be0227dc3054734e5f3
packfile.c: prevent overflow in `nth_packed_object_id()`

In 37fec86a83 (packfile: abstract away hash constant values,
2018-05-02), `nth_packed_object_id()` started using the variable
`the_hash_algo->rawsz` instead of a fixed constant when trying to
compute an offset into the ".idx" file for some object position.

This can lead to surprising truncation when looking for an object
towards the end of a large enough pack, like the following:

    (gdb) p hashsz
    $1 = 20
    (gdb) p n
    $2 = 215043814
    (gdb) p hashsz * n
    $3 = 5908984

, which is a debugger session broken on a known-bad call to the
`nth_packed_object_id()` function.

This behavior predates 37fec86a83, and is original to the v2 index
format, via: 74e34e1fca (sha1_file.c: learn about index version 2,
2007-04-09).

This is due to §6.4.4.1 of the C99 standard, which states that an
untyped integer constant will take the first type in which the value can
be accurately represented, among `int`, `long int`, and `long long int`.

Since 20 can be represented as an `int`, and `n` is a 32-bit unsigned
integer, the resulting computation is defined by §6.3.1.8, and the
(signed) integer value representing `n` is converted to an unsigned
type, meaning that `20 * n` (for `n` having type `uint32_t`) is
equivalent to a multiplication between two unsigned 32-bit integers.

When multiplying a sufficiently large `n`, the resulting value can
exceed 2^32-1, wrapping around and producing an invalid result. Let's
follow the example in f86f769550e (compute pack .idx byte offsets using
size_t, 2020-11-13) and replace this computation with `st_mult()`, which
will ensure that the computation is done using 64-bits.

While here, guard the corresponding computation for packs with v1
indexes, too. Though the likelihood of seeing a bug there is much
smaller, since (a) v1 indexes are generated far less frequently than v2
indexes, and (b) they all correspond to packs no larger than 2 GiB, so
having enough objects to trigger this overflow is unlikely if not
impossible.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
packfile.c