From 5e8c47ecb1065dede04bfbbeb76fa02851e52bb9 Mon Sep 17 00:00:00 2001 From: Vicki Pfau Date: Wed, 26 Feb 2025 23:44:49 -0800 Subject: [PATCH] Util: Cap internal buffer size when unzipping files (fixes #3404) --- CHANGES | 1 + src/util/vfs/vfs-zip.c | 61 ++++++++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index ef0dbe7fc..32c7fbd60 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,7 @@ Other fixes: - GBA Cheats: Let VBA-style codes patch ROM (fixes mgba.io/i/3423) - GBA Core: Fix booting into BIOS when skip BIOS is enabled - GBA Hardware: Fix loading states unconditionally overwriting GPIO memory + - Wii: Fix crash on loading large ZIP files (fixes mgba.io/i/3404) Misc: - GB: Allow use of CGB-E and AGB-0 BIOS versions (closes mgba.io/i/3427) - Updater: Fix rewriting folders and files on Windows (fixes mgba.io/i/3384) diff --git a/src/util/vfs/vfs-zip.c b/src/util/vfs/vfs-zip.c index 65e1c1e32..4233e8690 100644 --- a/src/util/vfs/vfs-zip.c +++ b/src/util/vfs/vfs-zip.c @@ -11,6 +11,12 @@ #ifdef USE_LIBZIP #include +#ifndef FIXED_ROM_BUFFER +#define MAX_BUFFER_SIZE 0x80000000 +#else +#define MAX_BUFFER_SIZE 0x00200000 +#endif + struct VDirEntryZip { struct VDirEntry d; struct zip* z; @@ -36,6 +42,7 @@ struct VFileZip { size_t fileSize; char* name; bool write; + size_t bufferStart; }; enum { @@ -250,8 +257,9 @@ bool _vfzClose(struct VFile* vf) { zip_source_free(source); return false; } - free(vfz->name); } + free(vfz->name); + vfz->name = NULL; if (vfz->zf && zip_fclose(vfz->zf) < 0) { return false; } @@ -286,6 +294,17 @@ off_t _vfzSeek(struct VFile* vf, off_t offset, int whence) { return -1; } + if (position < vfz->bufferStart) { + if (zip_fclose(vfz->zf) < 0) { + return -1; + } + vfz->zf = zip_fopen(vfz->z, vfz->name, 0); + vfz->bufferStart = 0; + vfz->offset = 0; + vfz->readSize = 0; + vfz->writeSize = 0; + } + if (position <= vfz->offset) { vfz->offset = position; return position; @@ -317,7 +336,7 @@ ssize_t _vfzRead(struct VFile* vf, void* buffer, size_t size) { while (bytesRead < size) { if (vfz->offset < vfz->readSize) { size_t diff = vfz->readSize - vfz->offset; - void* start = &((uint8_t*) vfz->buffer)[vfz->offset]; + void* start = &((uint8_t*) vfz->buffer)[vfz->offset - vfz->bufferStart]; if (diff > size - bytesRead) { diff = size - bytesRead; } @@ -332,16 +351,25 @@ ssize_t _vfzRead(struct VFile* vf, void* buffer, size_t size) { } } // offset == readSize - if (vfz->readSize == vfz->bufferSize) { - vfz->bufferSize *= 2; - if (vfz->bufferSize > vfz->fileSize) { - vfz->bufferSize = vfz->fileSize; + if (vfz->readSize == vfz->bufferSize + vfz->bufferStart) { + size_t bufferSize = vfz->bufferSize * 2; + void* newBuffer = NULL; + if (bufferSize <= MAX_BUFFER_SIZE) { + if (bufferSize > vfz->fileSize) { + bufferSize = vfz->fileSize; + } + newBuffer = realloc(vfz->buffer, bufferSize); + } + if (newBuffer) { + vfz->bufferSize = bufferSize; + vfz->buffer = newBuffer; + } else { + vfz->bufferStart += vfz->bufferSize; } - vfz->buffer = realloc(vfz->buffer, vfz->bufferSize); } - if (vfz->readSize < vfz->bufferSize) { - void* start = &((uint8_t*) vfz->buffer)[vfz->readSize]; - size_t toRead = vfz->bufferSize - vfz->readSize; + if (vfz->readSize < vfz->bufferSize + vfz->bufferStart) { + void* start = &((uint8_t*) vfz->buffer)[vfz->readSize - vfz->bufferStart]; + size_t toRead = vfz->bufferSize - vfz->readSize - vfz->bufferStart; if (toRead > BLOCK_SIZE) { toRead = BLOCK_SIZE; } @@ -393,9 +421,16 @@ void* _vfzMap(struct VFile* vf, size_t size, int flags) { struct VFileZip* vfz = (struct VFileZip*) vf; UNUSED(flags); + if (size > vfz->fileSize) { + return NULL; + } + size_t start = vfz->bufferStart; if (size > vfz->readSize) { vf->read(vf, 0, size - vfz->readSize); } + if (vfz->bufferStart != start) { + return NULL; + } return vfz->buffer; } @@ -472,10 +507,8 @@ struct VFile* _vdzOpenFile(struct VDir* vd, const char* path, int mode) { vfz->zf = zf; vfz->z = vdz->z; vfz->fileSize = s.size; - if ((mode & O_ACCMODE) == O_WRONLY) { - vfz->name = strdup(path); - vfz->write = true; - } + vfz->name = strdup(path); + vfz->write = (mode & O_ACCMODE) == O_WRONLY; vfz->d.close = _vfzClose; vfz->d.seek = _vfzSeek;