From 89b8c35ff4b5addc08a85bf5df02b407f8af1f6c Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 6 Apr 2025 22:34:37 +0200 Subject: [PATCH] Improve lseek handling (#2564) The skip functions are limited to 1 GB for cases in which libarchive runs on a system with an off_t or long with 32 bits. This has negative impact on 64 bit systems. Instead, make sure that _all_ subsequent functions truncate properly. Some of them already did and some had regressions for over 10 years. Tests pass on Debian 12 i686 configured with --disable-largefile, i.e. running with an off_t with 32 bits. Casts added where needed to still pass MSVC builds. --------- Signed-off-by: Tobias Stoeckmann CVE: CVE-2025-5918 Upstream-Status: Backport [https://github.com/libarchive/libarchive/commit/89b8c35ff4b5addc08a85bf5df02b407f8af1f6c] Signed-off-by: Divya Chellam --- libarchive/archive_read.c | 6 ---- libarchive/archive_read_disk_posix.c | 3 +- libarchive/archive_read_open_fd.c | 29 +++++++++++++------ libarchive/archive_read_open_file.c | 35 ++++++++++++----------- libarchive/archive_read_open_filename.c | 37 ++++++++++++++++++------- libarchive/test/read_open_memory.c | 2 +- libarchive/test/test_sparse_basic.c | 6 ++-- libarchive/test/test_tar_large.c | 2 +- 8 files changed, 75 insertions(+), 45 deletions(-) diff --git a/libarchive/archive_read.c b/libarchive/archive_read.c index 822c534..50db870 100644 --- a/libarchive/archive_read.c +++ b/libarchive/archive_read.c @@ -176,15 +176,9 @@ client_skip_proxy(struct archive_read_filter *self, int64_t request) return 0; if (self->archive->client.skipper != NULL) { - /* Seek requests over 1GiB are broken down into - * multiple seeks. This avoids overflows when the - * requests get passed through 32-bit arguments. */ - int64_t skip_limit = (int64_t)1 << 30; int64_t total = 0; for (;;) { int64_t get, ask = request; - if (ask > skip_limit) - ask = skip_limit; get = (self->archive->client.skipper) (&self->archive->archive, self->data, ask); total += get; diff --git a/libarchive/archive_read_disk_posix.c b/libarchive/archive_read_disk_posix.c index 09965eb..4839d62 100644 --- a/libarchive/archive_read_disk_posix.c +++ b/libarchive/archive_read_disk_posix.c @@ -778,7 +778,8 @@ _archive_read_data_block(struct archive *_a, const void **buff, */ if (t->current_sparse->offset > t->entry_total) { if (lseek(t->entry_fd, - (off_t)t->current_sparse->offset, SEEK_SET) < 0) { + (off_t)t->current_sparse->offset, SEEK_SET) != + t->current_sparse->offset) { archive_set_error(&a->archive, errno, "Seek error"); r = ARCHIVE_FATAL; a->archive.state = ARCHIVE_STATE_FATAL; diff --git a/libarchive/archive_read_open_fd.c b/libarchive/archive_read_open_fd.c index debfde2..3fd536d 100644 --- a/libarchive/archive_read_open_fd.c +++ b/libarchive/archive_read_open_fd.c @@ -131,7 +131,7 @@ static int64_t file_skip(struct archive *a, void *client_data, int64_t request) { struct read_fd_data *mine = (struct read_fd_data *)client_data; - int64_t skip = request; + off_t skip = (off_t)request; int64_t old_offset, new_offset; int skip_bits = sizeof(skip) * 8 - 1; /* off_t is a signed type. */ @@ -140,15 +140,15 @@ file_skip(struct archive *a, void *client_data, int64_t request) /* Reduce a request that would overflow the 'skip' variable. */ if (sizeof(request) > sizeof(skip)) { - int64_t max_skip = + const int64_t max_skip = (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1; if (request > max_skip) - skip = max_skip; + skip = (off_t)max_skip; } - /* Reduce request to the next smallest multiple of block_size */ - request = (request / mine->block_size) * mine->block_size; - if (request == 0) + /* Reduce 'skip' to the next smallest multiple of block_size */ + skip = (off_t)(((int64_t)skip / mine->block_size) * mine->block_size); + if (skip == 0) return (0); if (((old_offset = lseek(mine->fd, 0, SEEK_CUR)) >= 0) && @@ -178,11 +178,24 @@ static int64_t file_seek(struct archive *a, void *client_data, int64_t request, int whence) { struct read_fd_data *mine = (struct read_fd_data *)client_data; + off_t seek = (off_t)request; int64_t r; + int seek_bits = sizeof(seek) * 8 - 1; /* off_t is a signed type. */ /* We use off_t here because lseek() is declared that way. */ - /* See above for notes about when off_t is less than 64 bits. */ - r = lseek(mine->fd, request, whence); + + /* Reduce a request that would overflow the 'seek' variable. */ + if (sizeof(request) > sizeof(seek)) { + const int64_t max_seek = + (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1; + const int64_t min_seek = ~max_seek; + if (request > max_seek) + seek = (off_t)max_seek; + else if (request < min_seek) + seek = (off_t)min_seek; + } + + r = lseek(mine->fd, seek, whence); if (r >= 0) return r; diff --git a/libarchive/archive_read_open_file.c b/libarchive/archive_read_open_file.c index ecd56dc..2829b9a 100644 --- a/libarchive/archive_read_open_file.c +++ b/libarchive/archive_read_open_file.c @@ -145,7 +145,7 @@ FILE_skip(struct archive *a, void *client_data, int64_t request) /* If request is too big for a long or an off_t, reduce it. */ if (sizeof(request) > sizeof(skip)) { - int64_t max_skip = + const int64_t max_skip = (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1; if (request > max_skip) skip = max_skip; @@ -176,39 +176,42 @@ FILE_seek(struct archive *a, void *client_data, int64_t request, int whence) { struct read_FILE_data *mine = (struct read_FILE_data *)client_data; #if HAVE__FSEEKI64 - int64_t skip = request; + int64_t seek = request; #elif HAVE_FSEEKO - off_t skip = (off_t)request; + off_t seek = (off_t)request; #else - long skip = (long)request; + long seek = (long)request; #endif - int skip_bits = sizeof(skip) * 8 - 1; + int seek_bits = sizeof(seek) * 8 - 1; (void)a; /* UNUSED */ - /* If request is too big for a long or an off_t, reduce it. */ - if (sizeof(request) > sizeof(skip)) { - int64_t max_skip = - (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1; - if (request > max_skip) - skip = max_skip; + /* Reduce a request that would overflow the 'seek' variable. */ + if (sizeof(request) > sizeof(seek)) { + const int64_t max_seek = + (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1; + const int64_t min_seek = ~max_seek; + if (request > max_seek) + seek = max_seek; + else if (request < min_seek) + seek = min_seek; } #ifdef __ANDROID__ /* Newer Android versions have fseeko...to meditate. */ - int64_t ret = lseek(fileno(mine->f), skip, whence); + int64_t ret = lseek(fileno(mine->f), seek, whence); if (ret >= 0) { return ret; } #elif HAVE__FSEEKI64 - if (_fseeki64(mine->f, skip, whence) == 0) { + if (_fseeki64(mine->f, seek, whence) == 0) { return _ftelli64(mine->f); } #elif HAVE_FSEEKO - if (fseeko(mine->f, skip, whence) == 0) { + if (fseeko(mine->f, seek, whence) == 0) { return ftello(mine->f); } #else - if (fseek(mine->f, skip, whence) == 0) { + if (fseek(mine->f, seek, whence) == 0) { return ftell(mine->f); } #endif @@ -226,4 +229,4 @@ FILE_close(struct archive *a, void *client_data) free(mine->buffer); free(mine); return (ARCHIVE_OK); -} \ No newline at end of file +} diff --git a/libarchive/archive_read_open_filename.c b/libarchive/archive_read_open_filename.c index 05f0ffb..3894b15 100644 --- a/libarchive/archive_read_open_filename.c +++ b/libarchive/archive_read_open_filename.c @@ -479,20 +479,24 @@ file_skip_lseek(struct archive *a, void *client_data, int64_t request) struct read_file_data *mine = (struct read_file_data *)client_data; #if defined(_WIN32) && !defined(__CYGWIN__) /* We use _lseeki64() on Windows. */ - int64_t old_offset, new_offset; + int64_t old_offset, new_offset, skip = request; #else - off_t old_offset, new_offset; + off_t old_offset, new_offset, skip = (off_t)request; #endif + int skip_bits = sizeof(skip) * 8 - 1; /* We use off_t here because lseek() is declared that way. */ - /* TODO: Deal with case where off_t isn't 64 bits. - * This shouldn't be a problem on Linux or other POSIX - * systems, since the configuration logic for libarchive - * tries to obtain a 64-bit off_t. - */ + /* Reduce a request that would overflow the 'skip' variable. */ + if (sizeof(request) > sizeof(skip)) { + const int64_t max_skip = + (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1; + if (request > max_skip) + skip = max_skip; + } + if ((old_offset = lseek(mine->fd, 0, SEEK_CUR)) >= 0 && - (new_offset = lseek(mine->fd, request, SEEK_CUR)) >= 0) + (new_offset = lseek(mine->fd, skip, SEEK_CUR)) >= 0) return (new_offset - old_offset); /* If lseek() fails, don't bother trying again. */ @@ -540,11 +544,24 @@ static int64_t file_seek(struct archive *a, void *client_data, int64_t request, int whence) { struct read_file_data *mine = (struct read_file_data *)client_data; + off_t seek = (off_t)request; int64_t r; + int seek_bits = sizeof(seek) * 8 - 1; /* We use off_t here because lseek() is declared that way. */ - /* See above for notes about when off_t is less than 64 bits. */ - r = lseek(mine->fd, request, whence); + + /* Reduce a request that would overflow the 'seek' variable. */ + if (sizeof(request) > sizeof(seek)) { + const int64_t max_seek = + (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1; + const int64_t min_seek = ~max_seek; + if (request > max_seek) + seek = (off_t)max_seek; + else if (request < min_seek) + seek = (off_t)min_seek; + } + + r = lseek(mine->fd, seek, whence); if (r >= 0) return r; diff --git a/libarchive/test/read_open_memory.c b/libarchive/test/read_open_memory.c index 6d2468c..9262ab9 100644 --- a/libarchive/test/read_open_memory.c +++ b/libarchive/test/read_open_memory.c @@ -167,7 +167,7 @@ memory_read_skip(struct archive *a, void *client_data, int64_t skip) (void)a; /* UNUSED */ /* We can't skip by more than is available. */ - if ((off_t)skip > (off_t)(mine->end - mine->p)) + if (skip > mine->end - mine->p) skip = mine->end - mine->p; /* Always do small skips by prime amounts. */ if (skip > 71) diff --git a/libarchive/test/test_sparse_basic.c b/libarchive/test/test_sparse_basic.c index 23cde56..93710cb 100644 --- a/libarchive/test/test_sparse_basic.c +++ b/libarchive/test/test_sparse_basic.c @@ -608,7 +608,8 @@ DEFINE_TEST(test_sparse_basic) verify_sparse_file(a, "file2", sparse_file2, 20); /* Encoded non sparse; expect a data block but no sparse entries. */ verify_sparse_file(a, "file3", sparse_file3, 0); - verify_sparse_file(a, "file4", sparse_file4, 2); + if (sizeof(off_t) > 4) + verify_sparse_file(a, "file4", sparse_file4, 2); assertEqualInt(ARCHIVE_OK, archive_read_free(a)); @@ -635,7 +636,8 @@ DEFINE_TEST(test_sparse_basic) verify_sparse_file(a, "file1", sparse_file1, 0); verify_sparse_file(a, "file2", sparse_file2, 0); verify_sparse_file(a, "file3", sparse_file3, 0); - verify_sparse_file(a, "file4", sparse_file4, 0); + if (sizeof(off_t) > 4) + verify_sparse_file(a, "file4", sparse_file4, 0); assertEqualInt(ARCHIVE_OK, archive_read_free(a)); diff --git a/libarchive/test/test_tar_large.c b/libarchive/test/test_tar_large.c index c1f3791..1cde321 100644 --- a/libarchive/test/test_tar_large.c +++ b/libarchive/test/test_tar_large.c @@ -175,7 +175,7 @@ memory_read_skip(struct archive *a, void *_private, int64_t skip) } if (private->filebytes > 0) { if (private->filebytes < skip) - skip = (off_t)private->filebytes; + skip = private->filebytes; private->filebytes -= skip; } else { skip = 0; -- 2.40.0