From: Omar Polo Subject: convert got_pack filesize to off_t To: gameoftrees@openbsd.org Date: Mon, 24 Oct 2022 22:04:19 +0200 here's the last piece: this converts the filesize field of the struct got_pack from size_t to off_t. To test this, i've created a repo with a few big, randomly-generated files until `git repack -a -d' gave me a packfile bigger than 4G (on amd64), then copied the repo to an i386 and saw got fail to handle it. (actually i went a bit overboard and generated 6GB of packfile.) $ uname -a OpenBSD chiaki.home.lab 7.2 GENERIC.MP#417 i386 $ /usr/local/bin/got -V got 0.76 $ /usr/local/bin/got log -P | grep ^commit commit 5f7afb519822a72570b428b62cc62bf44c378226 (main) commit f3021112535346c8ff1cb63ce7a0e8968daf9302 commit 23e8d7613b956eb768032cca4ff04fa963ecf087 commit 35c4c26bcab6652c4c72e6166855230ab1ad5a0a got-read-pack: bad offset in pack file got: bad offset in pack file $ got log -P | grep ^commit commit 5f7afb519822a72570b428b62cc62bf44c378226 (main) commit f3021112535346c8ff1cb63ce7a0e8968daf9302 commit 23e8d7613b956eb768032cca4ff04fa963ecf087 commit 35c4c26bcab6652c4c72e6166855230ab1ad5a0a commit f3d677f2833ff4bad7abc4d6461124f128d895f7 commit 8a2fe22648044f8e20a4f43a370844c1196f96b1 commit a702fb23dd465bfc9538c28fb43cbacc010ae668 commit 47bb96a8ca78d72f0adf89d94057aa6c4fde0c14 commit 4de1d9c2deec76f8f591c5c63a8d4b067de88d6b commit 3a8e0626f15c337852b97ecf952ade40e48b4059 commit 55ccb88a64862a72bbf671099ce50e12b6c91920 This pushes the size_t truncation to inflate and the delta cache boundaries. Will follow-up with fixes for those too. diff 90ac69281429b4caf05f7b3617e0acabdc062028 42f21d333d52b0add5fe9b3007b2c3af178f373a commit - 90ac69281429b4caf05f7b3617e0acabdc062028 commit + 42f21d333d52b0add5fe9b3007b2c3af178f373a 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 - 0b79146e747f73787b2ee568b7fb74c517ce1d9e blob + deffa8b86d90c131ff29beea23ed1dd7c205a12a --- lib/pack.c +++ lib/pack.c @@ -819,6 +819,7 @@ got_pack_close(struct got_pack *pack) const struct got_error *err = NULL; err = pack_stop_privsep_child(pack); + /* XXX: check that pack->filesize < SIZE_MAX ? */ if (pack->map && munmap(pack->map, pack->filesize) == -1 && !err) err = got_error_from_errno("munmap"); if (pack->fd != -1 && close(pack->fd) == -1 && err == NULL) @@ -856,6 +857,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 +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); + } + mapoff = (size_t)delta_offset + *len; if (mapoff + sizeof(offN) >= pack->filesize) return got_error(GOT_ERR_PACK_OFFSET); @@ -1063,7 +1077,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 +1099,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", + 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 +1152,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 +1419,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 +1445,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 +1610,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 +1631,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 +1758,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 +1796,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 + 01b3c9d4d9ae02b123a6e3e2ffb9a5b0b14ea103 --- 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, @@ -319,6 +325,13 @@ read_packed_object(struct got_pack *pack, struct got_i err = got_error(GOT_ERR_BAD_PACKFILE); 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); @@ -797,6 +810,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 - e914ae79dd30bfd6591ea94dfadb6d91ddd80c65 blob + b73c1dfc1e76b5ddb1d19e42a225ec6a0c1a7c8b --- 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");