[PATCH 0/2] archive, apply: do not truncate files of 4GiB or more

[PATCH 0/2] archive, apply: do not truncate files of 4GiB or more

[Elia Pinto]

Two more cases where, on platforms where "long" is 32 bits but size_t
is 64 bits (64-bit Windows), a file of 4GiB or more is written truncated
or corrupt without any error.

- 1/2 archive: tar and zip of a file written by streaming come out
  corrupt.
- 2/2 apply: the file being patched is written truncated with exit 0
  (also with --index and git am).

No new tests; both were verified by simulating the truncation at 16
bits, as described in the messages.

Regarding t5004, the existing test "zip archive with files bigger than
4GB" has been excluded where "long" is 32 bits (LONG_IS_64BIT) since
2017, because back then the object database couldn't read objects that
large. Today on 64-bit Windows it can, and with this series zip works
too. Thus, the guard could become SIZE_T_IS_64BIT to enable a real test
on Windows. Since I haven't run it on Windows, I'm offering this as a
follow-up or asking reviewers for an opinion.

With this series, the search for these truncations is finished. The
remaining warnings are not reachable: a 4GiB index, sizes bounded by
config or by the format, commits over 4GiB that already die.

It doesn't depend on the other series in flight and merges cleanly with
them. Based on master 50fe5cc63f.

Elia Pinto (2):
  archive: use size_t for the size of archive entries
  apply: use size_t for the size of the result written out

 apply.c       |  8 ++++----
 archive-tar.c | 10 +++++-----
 archive-zip.c | 16 ++++++++--------
 archive.h     |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)


base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
2.53.0

[PATCH 1/2] archive: use size_t for the size of archive entries

[Elia Pinto]

The archive entry callback and format implementations use "unsigned
long" for the entry size, while blobs above core.bigFileThreshold
arrive as size_t.

On platforms where "long" is 32 bits but size_t is 64 bits, such as
64-bit Windows, streaming a file of 4GiB or more causes silent
corruption: for tar, the truncated size goes into the ustar header while
stream_blocked() writes the entire blob, corrupting the archive; for
zip, write_zip_entry() overflows compressed_size, producing incorrect
headers and a corrupted archive.

This was verified using a 16-bit simulation, where tar extracted
truncated data and Python's zipfile rejected the output due to bad
magic numbers or CRC-32 errors.

Use size_t in the callback, tar writer, and zip writer functions,
allowing the existing zip64 helpers to handle large files correctly.

Triggering this requires processing a file of 4GiB or more on a platform
where "long" is 32 bits but size_t is 64 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 archive-tar.c | 10 +++++-----
 archive-zip.c | 16 ++++++++--------
 archive.h     |  2 +-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index a1c66024d4..31ab704886 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -66,7 +66,7 @@ static void write_if_needed(void)
  * queues up writes, so that all our write(2) calls write exactly one
  * full block; pads writes to RECORDSIZE
  */
-static void do_write_blocked(const void *data, unsigned long size)
+static void do_write_blocked(const void *data, size_t size)
 {
 	const char *buf = data;
 
@@ -102,7 +102,7 @@ static void finish_record(void)
 	write_if_needed();
 }
 
-static void write_blocked(const void *data, unsigned long size)
+static void write_blocked(const void *data, size_t size)
 {
 	do_write_blocked(data, size);
 	finish_record();
@@ -218,7 +218,7 @@ static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)
 
 static void prepare_header(struct archiver_args *args,
 			   struct ustar_header *header,
-			   unsigned int mode, unsigned long size)
+			   unsigned int mode, size_t size)
 {
 	xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 07777);
 	xsnprintf(header->size, sizeof(header->size), "%011"PRIoMAX , S_ISREG(mode) ? (uintmax_t)size : (uintmax_t)0);
@@ -256,11 +256,11 @@ static int write_tar_entry(struct archiver_args *args,
 			   const struct object_id *oid,
 			   const char *path, size_t pathlen,
 			   unsigned int mode,
-			   void *buffer, unsigned long size)
+			   void *buffer, size_t size)
 {
 	struct ustar_header header;
 	struct strbuf ext_header = STRBUF_INIT;
-	unsigned long size_in_header;
+	size_t size_in_header;
 	int err = 0;
 
 	memset(&header, 0, sizeof(header));
diff --git a/archive-zip.c b/archive-zip.c
index 785689584c..2ca6dd956d 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -191,9 +191,9 @@ static uint32_t clamp32(uintmax_t n)
 	return (n < max) ? n : max;
 }
 
-static void *zlib_deflate_raw(void *data, unsigned long size,
+static void *zlib_deflate_raw(void *data, size_t size,
 			      int compression_level,
-			      unsigned long *compressed_size)
+			      size_t *compressed_size)
 {
 	git_zstream stream;
 	size_t maxsize;
@@ -224,8 +224,8 @@ static void *zlib_deflate_raw(void *data, unsigned long size,
 	return buffer;
 }
 
-static void write_zip_data_desc(unsigned long size,
-				unsigned long compressed_size,
+static void write_zip_data_desc(size_t size,
+				size_t compressed_size,
 				unsigned long crc)
 {
 	if (size >= 0xffffffff || compressed_size >= 0xffffffff) {
@@ -248,8 +248,8 @@ static void write_zip_data_desc(unsigned long size,
 }
 
 static void set_zip_header_data_desc(struct zip_local_header *header,
-				     unsigned long size,
-				     unsigned long compressed_size,
+				     size_t size,
+				     size_t compressed_size,
 				     unsigned long crc)
 {
 	copy_le32(header->crc32, crc);
@@ -285,7 +285,7 @@ static int write_zip_entry(struct archiver_args *args,
 			   const struct object_id *oid,
 			   const char *path, size_t pathlen,
 			   unsigned int mode,
-			   void *buffer, unsigned long size)
+			   void *buffer, size_t size)
 {
 	struct zip_local_header header;
 	uintmax_t offset = zip_offset;
@@ -293,7 +293,7 @@ static int write_zip_entry(struct archiver_args *args,
 	size_t header_extra_size = zip_extra_mtime.len;
 	int need_zip64_extra = 0;
 	unsigned long attr2;
-	unsigned long compressed_size;
+	size_t compressed_size;
 	unsigned long crc;
 	enum zip_method method;
 	unsigned char *out;
diff --git a/archive.h b/archive.h
index bbe65ba0f9..e52328d639 100644
--- a/archive.h
+++ b/archive.h
@@ -56,7 +56,7 @@ typedef int (*write_archive_entry_fn_t)(struct archiver_args *args,
 					const struct object_id *oid,
 					const char *path, size_t pathlen,
 					unsigned int mode,
-					void *buffer, unsigned long size);
+					void *buffer, size_t size);
 
 int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
 
-- 
2.53.0

[PATCH 2/2] apply: use size_t for the size of the result written out

[Elia Pinto]

While "git apply" limits the patch size, the result size is stored as
size_t but passed to functions using "unsigned long size".

On platforms where "long" is 32 bits but size_t is 64 bits, such as
64-bit Windows, applying a patch to a file of 4GiB or more causes the
result to be written out truncated with exit 0, silently losing data;
this also affects "git am" and "git apply --index", which writes the
truncated content to the index.

This was verified using a 16-bit simulation, where "git apply" left
a 7901-line file truncated to 2859 lines.

Use size_t in create_file(), create_one_file(), try_create_file(), and
add_index_file(). add_index_file() passes the size on to
odb_write_object(), which takes a size_t with the "use size_t for a few
more object and buffer sizes" series.

Triggering this requires processing a file of 4GiB or more on a platform
where "long" is 32 bits but size_t is 64 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 apply.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/apply.c b/apply.c
index a0b7119c63..02d6791ad7 100644
--- a/apply.c
+++ b/apply.c
@@ -4458,7 +4458,7 @@ static int add_index_file(struct apply_state *state,
 			  const char *path,
 			  unsigned mode,
 			  void *buf,
-			  unsigned long size)
+			  size_t size)
 {
 	struct stat st;
 	struct cache_entry *ce;
@@ -4513,7 +4513,7 @@ static int add_index_file(struct apply_state *state,
  */
 static int try_create_file(struct apply_state *state, const char *path,
 			   unsigned int mode, const char *buf,
-			   unsigned long size)
+			   size_t size)
 {
 	int fd, res;
 	struct strbuf nbuf = STRBUF_INIT;
@@ -4564,7 +4564,7 @@ static int create_one_file(struct apply_state *state,
 			   char *path,
 			   unsigned mode,
 			   const char *buf,
-			   unsigned long size)
+			   size_t size)
 {
 	char *newpath = NULL;
 	int res;
@@ -4686,7 +4686,7 @@ static int create_file(struct apply_state *state, struct patch *patch)
 {
 	char *path = patch->new_name;
 	unsigned mode = patch->new_mode;
-	unsigned long size = patch->resultsize;
+	size_t size = patch->resultsize;
 	char *buf = patch->result;
 
 	if (!mode)
-- 
2.53.0