"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: convert got_pack filesize to off_t
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 25 Oct 2022 12:13:21 +0200

Download raw body.

Thread
On 2022/10/25 10:05:32 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > @@ -935,6 +942,13 @@ parse_negative_offset(int64_t *offset, size_t *len, st
> >  
> >  		if (pack->map) {
> >  			size_t mapoff;
> > +
> > +			if (delta_offset + *len > SIZE_MAX) {
> > +				return got_error_fmt(GOT_ERR_PACK_OFFSET,
> > +				    "mapoff %lld would overflow size_t",
> > +				    delta_offset + *len);
> 
> Should this cast to (long long) as per naddy's commit e082ed6708?

Yep!

-----------------------------------------------
commit a9b01bbc1720076cac56989f3932f9b249b83619 (siz)
from: Omar Polo <op@omarpolo.com>
date: Tue Oct 25 10:11:17 2022 UTC
 
 convert got_pack' filesize to off_t
 
diff 1c28a36116110db5de40e6edf09887651f3ca37b a9b01bbc1720076cac56989f3932f9b249b83619
commit - 1c28a36116110db5de40e6edf09887651f3ca37b
commit + a9b01bbc1720076cac56989f3932f9b249b83619
blob - 51a31d0b8608b9351cb8b63bfa93e3d5ab4f87f8
blob + 7b157b7e62f061b3d5da454b7ae3cb321862e407
--- gotd/repo_write.c
+++ gotd/repo_write.c
@@ -999,15 +999,10 @@ recv_packfile(struct repo_write_client **client, struc
 
 	log_debug("pack data received");
 
-	/* XXX size_t vs off_t, both should be off_t */
-	if (pack_filesize >= SIZE_MAX) {
-		err = got_error_msg(GOT_ERR_BAD_PACKFILE,
-		    "pack file too large");
-		goto done;
-	}
 	pack->filesize = pack_filesize;
 
-	log_debug("begin indexing pack (%zu bytes in size)", pack->filesize);
+	log_debug("begin indexing pack (%lld bytes in size)",
+	    (long long)pack->filesize);
 	err = got_pack_index(pack, (*client)->packidx_fd,
 	    tempfiles[0], tempfiles[1], tempfiles[2], (*client)->pack_sha1,
 	    pack_index_progress, NULL, &rl);
blob - 38d0cdc9b37e656d62eca1ed1def96ff6e7e2ef5
blob + 00518379da8d1ea10fa247bbd516547bf00019e1
--- lib/got_lib_pack.h
+++ lib/got_lib_pack.h
@@ -25,7 +25,7 @@ struct got_pack {
 	char *path_packfile;
 	int fd;
 	uint8_t *map;
-	size_t filesize;
+	off_t filesize;
 	struct got_pack_privsep_child *privsep_child;
 	int basefd;
 	int accumfd;
blob - 1d66ed4d59e710d6c7937f42faafc674ea675e66
blob + f84d920be3d90fdc2c9b6db77113efd0d7d87c62
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -514,7 +514,7 @@ struct got_imsg_pack {
 /* Structure for GOT_IMSG_PACK. */
 struct got_imsg_pack {
 	char path_packfile[PATH_MAX];
-	size_t filesize;
+	off_t filesize;
 	/* Additionally, a file desciptor is passed via imsg. */
 } __attribute__((__packed__));
 
blob - b043d4d76bcee830e606273e62636fe996c1d756
blob + 36efe8c30ade7284694bc600951d79771cc42662
--- lib/pack.c
+++ lib/pack.c
@@ -856,6 +856,12 @@ got_pack_parse_object_type_and_size(uint8_t *type, uin
 		return got_error(GOT_ERR_PACK_OFFSET);
 
 	if (pack->map) {
+		if (offset > SIZE_MAX) {
+			return got_error_fmt(GOT_ERR_PACK_OFFSET,
+			    "offset %lld overflows size_t",
+			    (long long)offset);
+		}
+
 		mapoff = (size_t)offset;
 	} else {
 		if (lseek(pack->fd, offset, SEEK_SET) == -1)
@@ -935,6 +941,13 @@ parse_negative_offset(int64_t *offset, size_t *len, st
 
 		if (pack->map) {
 			size_t mapoff;
+
+			if (delta_offset + *len > SIZE_MAX) {
+				return got_error_fmt(GOT_ERR_PACK_OFFSET,
+				    "mapoff %lld would overflow size_t",
+				    (long long)delta_offset + *len);
+			}
+
 			mapoff = (size_t)delta_offset + *len;
 			if (mapoff + sizeof(offN) >= pack->filesize)
 				return got_error(GOT_ERR_PACK_OFFSET);
@@ -1063,7 +1076,7 @@ resolve_offset_delta(struct got_delta_chain *deltas,
 	}
 
 	err = add_delta(deltas, delta_offset, tslen, delta_type, delta_size,
-	    delta_data_offset);
+	    delta_data_offset);	/* XXX: off_t vs size_t! */
 	if (err)
 		return err;
 
@@ -1085,7 +1098,15 @@ got_pack_parse_ref_delta(struct got_object_id *id,
     struct got_pack *pack, off_t delta_offset, int tslen)
 {
 	if (pack->map) {
-		size_t mapoff = delta_offset + tslen;
+		size_t mapoff;
+
+		if (delta_offset + tslen > SIZE_MAX) {
+			return got_error_fmt(GOT_ERR_PACK_OFFSET,
+			    "mapoff %lld would overflow size_t",
+			    (long long)delta_offset + tslen);
+		}
+
+		mapoff = delta_offset + tslen;
 		if (mapoff + sizeof(*id) >= pack->filesize)
 			return got_error(GOT_ERR_PACK_OFFSET);
 		memcpy(id, pack->map + mapoff, sizeof(*id));
@@ -1130,7 +1151,7 @@ resolve_ref_delta(struct got_delta_chain *deltas, stru
 	}
 
 	err = add_delta(deltas, delta_offset, tslen, delta_type, delta_size,
-	    delta_data_offset);
+	    delta_data_offset);	/* XXX: off_t vs size_t */
 	if (err)
 		return err;
 
@@ -1397,7 +1418,16 @@ got_pack_dump_delta_chain_to_file(size_t *result_size,
 				max_size = delta->size;
 			if (max_size > max_bufsize) {
 				if (pack->map) {
-					mapoff = (size_t)delta_data_offset;
+					if (delta_data_offset > SIZE_MAX) {
+						return got_error_fmt(
+						    GOT_ERR_RANGE,
+						    "delta offset %lld "
+						    "overflows size_t",
+						    (long long)
+						    delta_data_offset);
+					}
+
+					mapoff = delta_data_offset;
 					err = got_inflate_to_file_mmap(
 					    &base_bufsz, NULL, NULL, pack->map,
 					    mapoff, pack->filesize - mapoff,
@@ -1414,7 +1444,16 @@ got_pack_dump_delta_chain_to_file(size_t *result_size,
 				}
 				accum_bufsz = max_size;
 				if (pack->map) {
-					mapoff = (size_t)delta_data_offset;
+					if (delta_data_offset > SIZE_MAX) {
+						return got_error_fmt(
+						    GOT_ERR_RANGE,
+						    "delta offset %lld "
+						    "overflows size_t",
+						    (long long)
+						    delta_data_offset);
+					}
+
+					mapoff = delta_data_offset;
 					err = got_inflate_to_mem_mmap(&base_buf,
 					    &base_bufsz, NULL, NULL,
 					    pack->map, mapoff,
@@ -1570,7 +1609,7 @@ got_pack_dump_delta_chain_to_mem(uint8_t **outbuf, siz
 		uint64_t base_size, result_size = 0;
 		int cached = 1;
 		if (n == 0) {
-			size_t delta_data_offset;
+			off_t delta_data_offset;
 
 			/* Plain object types are the delta base. */
 			if (delta->type != GOT_OBJ_TYPE_COMMIT &&
@@ -1591,7 +1630,16 @@ got_pack_dump_delta_chain_to_mem(uint8_t **outbuf, siz
 				max_size = delta->size;
 
 			if (pack->map) {
-				size_t mapoff = (size_t)delta_data_offset;
+				size_t mapoff;
+
+				if (delta_data_offset > SIZE_MAX) {
+					return got_error_fmt(GOT_ERR_RANGE,
+					    "delta %lld offset would "
+					    "overflow size_t",
+					    (long long)delta_data_offset);
+				}
+
+				mapoff = delta_data_offset;
 				err = got_inflate_to_mem_mmap(&base_buf,
 				    &base_bufsz, NULL, NULL, pack->map,
 				    mapoff, pack->filesize - mapoff);
@@ -1709,7 +1757,15 @@ got_packfile_extract_object(struct got_pack *pack, str
 			return got_error(GOT_ERR_PACK_OFFSET);
 
 		if (pack->map) {
-			size_t mapoff = (size_t)obj->pack_offset;
+			size_t mapoff;
+
+			if (obj->pack_offset > SIZE_MAX) {
+				return got_error_fmt(GOT_ERR_RANGE,
+				    "pack offset %lld would overflow size_t",
+				    (long long)obj->pack_offset);
+			}
+
+			mapoff = obj->pack_offset;
 			err = got_inflate_to_file_mmap(&obj->size, NULL, NULL,
 			    pack->map, mapoff, pack->filesize - mapoff,
 			    outfile);
@@ -1739,7 +1795,15 @@ got_packfile_extract_object_to_mem(uint8_t **buf, size
 		if (obj->pack_offset >= pack->filesize)
 			return got_error(GOT_ERR_PACK_OFFSET);
 		if (pack->map) {
-			size_t mapoff = (size_t)obj->pack_offset;
+			size_t mapoff;
+
+			if (obj->pack_offset > SIZE_MAX) {
+				return got_error_fmt(GOT_ERR_RANGE,
+				    "pack offset %lld would overflow size_t",
+				    (long long)obj->pack_offset);
+			}
+
+			mapoff = obj->pack_offset;
 			err = got_inflate_to_mem_mmap(buf, len, NULL, NULL,
 			    pack->map, mapoff, pack->filesize - mapoff);
 		} else {
blob - 07b401c3fd8743bb46cd007e16800e79aaa387dc
blob + 8d1f70836b3c957b385dad80e0bc73f6739b9089
--- lib/pack_index.c
+++ lib/pack_index.c
@@ -271,6 +271,12 @@ read_packed_object(struct got_pack *pack, struct got_i
 				err = got_error(GOT_ERR_BAD_PACKFILE);
 				break;
 			}
+			if (mapoff + SHA1_DIGEST_LENGTH > SIZE_MAX) {
+				err = got_error_fmt(GOT_ERR_RANGE,
+				    "mapoff %lld would overflow size_t",
+				    (long long)mapoff + SHA1_DIGEST_LENGTH);
+				break;
+			}
 			memcpy(obj->delta.ref.ref_id.sha1, pack->map + mapoff,
 			    SHA1_DIGEST_LENGTH);
 			obj->crc = crc32(obj->crc, pack->map + mapoff,
@@ -320,6 +326,14 @@ read_packed_object(struct got_pack *pack, struct got_i
 				break;
 			}
 
+			if (mapoff + obj->delta.ofs.base_offsetlen >
+			    SIZE_MAX) {
+				err = got_error_fmt(GOT_ERR_RANGE,
+				    "mapoff %lld would overflow size_t",
+				    (long long)mapoff
+				    + obj->delta.ofs.base_offsetlen);
+			}
+
 			obj->crc = crc32(obj->crc, pack->map + mapoff,
 			    obj->delta.ofs.base_offsetlen);
 			SHA1Update(pack_sha1_ctx, pack->map + mapoff,
@@ -797,6 +811,13 @@ got_pack_index(struct got_pack *pack, int idxfd, FILE 
 
 	/* Verify the SHA1 checksum stored at the end of the pack file. */
 	if (pack->map) {
+		if (pack->filesize > SIZE_MAX) {
+			err = got_error_fmt(GOT_ERR_RANGE,
+			    "filesize %lld overflows size_t",
+			    (long long)pack->filesize);
+			goto done;
+		}
+
 		memcpy(pack_sha1_expected, pack->map +
 		    pack->filesize - SHA1_DIGEST_LENGTH,
 		    SHA1_DIGEST_LENGTH);
blob - 7fda0c4e47f3539c4b13427d388cc26e749b75bb
blob + e44cf3100a855c7702d3a6f0abd5af9889166e60
--- libexec/got-index-pack/got-index-pack.c
+++ libexec/got-index-pack/got-index-pack.c
@@ -175,7 +175,7 @@ main(int argc, char **argv)
 		err = got_error_from_errno("lseek");
 		goto done;
 	}
-	pack.filesize = packfile_size; /* XXX off_t vs size_t */
+	pack.filesize = packfile_size;
 
 	if (lseek(pack.fd, 0, SEEK_SET) == -1) {
 		err = got_error_from_errno("lseek");