index-pack, unpack-objects: use size_t for object size

When unpacking objects from a packfile, the object size is decoded
from a variable-length encoding. On platforms where unsigned long is
32-bit (such as Windows, even in 64-bit builds), the shift operation
overflows when decoding sizes larger than 4GB. The result is a
truncated size value, causing the unpacked object to be corrupted or
rejected.

Fix this by changing the size variable to size_t, which is 64-bit on
64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
space.

Declare the per-byte continuation variable `c` as size_t as well,
matching the canonical varint decoder unpack_object_header_buffer()
in packfile.c. With c as size_t the expression (c & 0x7f) << shift
is naturally size_t-typed, so the explicit cast that an earlier
iteration carried at the use site is no longer needed.

While at it, add the same overflow guard that
unpack_object_header_buffer() carries: if the cumulative shift would
exceed bitsizeof(size_t) - 7, refuse the input rather than invoking
undefined behavior. Unlike unpack_object_header_buffer(), which
labels this case "bad object header", report it as the platform
limit it actually is: a header may be perfectly well-formed and
still encode a size we cannot represent locally (notably on a
32-bit build consuming a packfile produced on a 64-bit host).

This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c496978322f1d7a3469013e21368c5a..2e4b42fa122ee211c3f78f9f97f0e3d492a494a4 100644 (file)
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -37,7 +37,7 @@ static const char index_pack_usage[] =
 
 struct object_entry {
        struct pack_idx_entry idx;
-       unsigned long size;
+       size_t size;
        unsigned char hdr_size;
        signed char type;
        signed char real_type;
@@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type)
        return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
 }
 
-static void *unpack_entry_data(off_t offset, unsigned long size,
+static void *unpack_entry_data(off_t offset, size_t size,
                               enum object_type type, struct object_id *oid)
 {
        static char fixed_buf[8192];
@@ -524,7 +524,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
                              struct object_id *oid)
 {
        unsigned char *p;
-       unsigned long size, c;
+       size_t size, c;
        off_t base_offset;
        unsigned shift;
        void *data;
@@ -539,6 +539,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
        size = (c & 15);
        shift = 4;
        while (c & 0x80) {
+               if ((bitsizeof(size_t) - 7) < shift)
+                       die(_("object size too large for this platform"));
                p = fill(1);
                c = *p;
                use(1);

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360f6d1f523c04a0aee98a2bccf378ac0..76b3d0dee3b1821b3d774e3d707beb06c98e6147 100644 (file)
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -533,7 +533,7 @@ static void unpack_one(unsigned nr)
 {
        unsigned shift;
        unsigned char *pack;
-       unsigned long size, c;
+       size_t size, c;
        enum object_type type;
 
        obj_list[nr].offset = consumed_bytes;
@@ -545,6 +545,8 @@ static void unpack_one(unsigned nr)
        size = (c & 15);
        shift = 4;
        while (c & 0x80) {
+               if ((bitsizeof(size_t) - 7) < shift)
+                       die(_("object size too large for this platform"));
                pack = fill(1);
                c = *pack;
                use(1);