Fix buffer over-read and memory leaks when using long filepaths in a zip archive and improved robustness of long filepaths and reading files.

This commit is contained in:
Mack 2022-12-06 17:40:46 +00:00
parent 83d5cbf519
commit 1326b7e04f
7 changed files with 110 additions and 45 deletions

View File

@ -30,6 +30,48 @@
#include "zip_io.h"
#include "core/templates/local_vector.h"
int godot_unzip_get_current_file_info(unzFile p_zip_file, unz_file_info64 &r_file_info, String &r_filepath) {
const uLong short_file_path_buffer_size = 16384ul;
char short_file_path_buffer[short_file_path_buffer_size];
int err = unzGetCurrentFileInfo64(p_zip_file, &r_file_info, short_file_path_buffer, short_file_path_buffer_size, nullptr, 0, nullptr, 0);
if (unlikely((err != UNZ_OK) || (r_file_info.size_filename > short_file_path_buffer_size))) {
LocalVector<char> long_file_path_buffer;
long_file_path_buffer.resize(r_file_info.size_filename);
err = unzGetCurrentFileInfo64(p_zip_file, &r_file_info, long_file_path_buffer.ptr(), long_file_path_buffer.size(), nullptr, 0, nullptr, 0);
if (err != UNZ_OK) {
return err;
}
r_filepath = String::utf8(long_file_path_buffer.ptr(), r_file_info.size_filename);
} else {
r_filepath = String::utf8(short_file_path_buffer, r_file_info.size_filename);
}
return err;
}
int godot_unzip_locate_file(unzFile p_zip_file, String p_filepath, bool p_case_sensitive) {
int err = unzGoToFirstFile(p_zip_file);
while (err == UNZ_OK) {
unz_file_info64 current_file_info;
String current_filepath;
err = godot_unzip_get_current_file_info(p_zip_file, current_file_info, current_filepath);
if (err == UNZ_OK) {
bool filepaths_are_equal = p_case_sensitive ? (p_filepath == current_filepath) : (p_filepath.nocasecmp_to(current_filepath) == 0);
if (filepaths_are_equal) {
return UNZ_OK;
}
err = unzGoToNextFile(p_zip_file);
}
}
return err;
}
//
void *zipio_open(voidpf opaque, const char *p_fname, int mode) {
Ref<FileAccess> *fa = reinterpret_cast<Ref<FileAccess> *>(opaque);
ERR_FAIL_COND_V(fa == nullptr, nullptr);
@ -38,17 +80,17 @@ void *zipio_open(voidpf opaque, const char *p_fname, int mode) {
fname.parse_utf8(p_fname);
int file_access_mode = 0;
if (mode & ZLIB_FILEFUNC_MODE_WRITE) {
file_access_mode |= FileAccess::WRITE;
}
if (mode & ZLIB_FILEFUNC_MODE_READ) {
file_access_mode |= FileAccess::READ;
}
if (mode & ZLIB_FILEFUNC_MODE_WRITE) {
file_access_mode |= FileAccess::WRITE;
}
if (mode & ZLIB_FILEFUNC_MODE_CREATE) {
file_access_mode |= FileAccess::WRITE_READ;
}
(*fa) = FileAccess::open(fname, file_access_mode);
(*fa) = FileAccess::open(fname, file_access_mode);
if (fa->is_null()) {
return nullptr;
}

View File

@ -39,6 +39,13 @@
#include "thirdparty/minizip/unzip.h"
#include "thirdparty/minizip/zip.h"
// Get the current file info and safely convert the full filepath to a String.
int godot_unzip_get_current_file_info(unzFile p_zip_file, unz_file_info64 &r_file_info, String &r_filepath);
// Try to locate the file in the archive specified by the filepath (works with large paths and Unicode).
int godot_unzip_locate_file(unzFile p_zip_file, String p_filepath, bool p_case_sensitive = true);
//
void *zipio_open(voidpf opaque, const char *p_fname, int mode);
uLong zipio_read(voidpf opaque, voidpf stream, void *buf, uLong size);
uLong zipio_write(voidpf opaque, voidpf stream, const void *buf, uLong size);

View File

@ -63,10 +63,13 @@
</methods>
<constants>
<constant name="APPEND_CREATE" value="0" enum="ZipAppend">
Create a new zip archive at the given path.
</constant>
<constant name="APPEND_CREATEAFTER" value="1" enum="ZipAppend">
Append a new zip archive to the end of the already existing file at the given path.
</constant>
<constant name="APPEND_ADDINZIP" value="2" enum="ZipAppend">
Add new files to the existing zip archive at the given path.
</constant>
</constants>
</class>

View File

@ -39,17 +39,19 @@ Error ZIPPacker::open(String p_path, ZipAppend p_append) {
}
zlib_filefunc_def io = zipio_create_io(&fa);
zf = zipOpen2(p_path.utf8().get_data(), p_append, NULL, &io);
return zf != NULL ? OK : FAILED;
zf = zipOpen2(p_path.utf8().get_data(), p_append, nullptr, &io);
return zf != nullptr ? OK : FAILED;
}
Error ZIPPacker::close() {
ERR_FAIL_COND_V_MSG(fa.is_null(), FAILED, "ZIPPacker cannot be closed because it is not open.");
Error err = zipClose(zf, NULL) == ZIP_OK ? OK : FAILED;
Error err = zipClose(zf, nullptr) == ZIP_OK ? OK : FAILED;
if (err == OK) {
zf = NULL;
DEV_ASSERT(fa == nullptr);
zf = nullptr;
}
return err;
}
@ -60,18 +62,18 @@ Error ZIPPacker::start_file(String p_path) {
OS::DateTime time = OS::get_singleton()->get_datetime();
zipfi.tmz_date.tm_sec = time.second;
zipfi.tmz_date.tm_min = time.minute;
zipfi.tmz_date.tm_hour = time.hour;
zipfi.tmz_date.tm_mday = time.day;
zipfi.tmz_date.tm_min = time.minute;
zipfi.tmz_date.tm_mon = time.month - 1;
zipfi.tmz_date.tm_sec = time.second;
zipfi.tmz_date.tm_year = time.year;
zipfi.dosDate = 0;
zipfi.external_fa = 0;
zipfi.internal_fa = 0;
zipfi.external_fa = 0;
int ret = zipOpenNewFileInZip(zf, p_path.utf8().get_data(), &zipfi, NULL, 0, NULL, 0, NULL, Z_DEFLATED, Z_DEFAULT_COMPRESSION);
return ret == ZIP_OK ? OK : FAILED;
int err = zipOpenNewFileInZip(zf, p_path.utf8().get_data(), &zipfi, nullptr, 0, nullptr, 0, nullptr, Z_DEFLATED, Z_DEFAULT_COMPRESSION);
return err == ZIP_OK ? OK : FAILED;
}
Error ZIPPacker::write_file(Vector<uint8_t> p_data) {

View File

@ -40,7 +40,7 @@ class ZIPPacker : public RefCounted {
GDCLASS(ZIPPacker, RefCounted);
Ref<FileAccess> fa;
zipFile zf;
zipFile zf = nullptr;
protected:
static void _bind_methods();

View File

@ -40,38 +40,35 @@ Error ZIPReader::open(String p_path) {
zlib_filefunc_def io = zipio_create_io(&fa);
uzf = unzOpen2(p_path.utf8().get_data(), &io);
return uzf != NULL ? OK : FAILED;
return uzf != nullptr ? OK : FAILED;
}
Error ZIPReader::close() {
ERR_FAIL_COND_V_MSG(fa.is_null(), FAILED, "ZIPReader cannot be closed because it is not open.");
return unzClose(uzf) == UNZ_OK ? OK : FAILED;
Error err = unzClose(uzf) == UNZ_OK ? OK : FAILED;
if (err == OK) {
DEV_ASSERT(fa == nullptr);
uzf = nullptr;
}
return err;
}
PackedStringArray ZIPReader::get_files() {
ERR_FAIL_COND_V_MSG(fa.is_null(), PackedStringArray(), "ZIPReader must be opened before use.");
int err = unzGoToFirstFile(uzf);
ERR_FAIL_COND_V(err != UNZ_OK, PackedStringArray());
List<String> s;
if (unzGoToFirstFile(uzf) != UNZ_OK) {
return PackedStringArray();
}
do {
unz_file_info64 file_info;
char filename[256]; // Note filename is a path !
int err = unzGetCurrentFileInfo64(uzf, &file_info, filename, sizeof(filename), NULL, 0, NULL, 0);
String filepath;
err = godot_unzip_get_current_file_info(uzf, file_info, filepath);
if (err == UNZ_OK) {
s.push_back(filename);
} else {
// Assume filename buffer was too small
char *long_filename_buff = (char *)memalloc(file_info.size_filename);
int err2 = unzGetCurrentFileInfo64(uzf, NULL, long_filename_buff, sizeof(long_filename_buff), NULL, 0, NULL, 0);
if (err2 == UNZ_OK) {
s.push_back(long_filename_buff);
memfree(long_filename_buff);
}
s.push_back(filepath);
}
} while (unzGoToNextFile(uzf) == UNZ_OK);
@ -87,23 +84,37 @@ PackedStringArray ZIPReader::get_files() {
PackedByteArray ZIPReader::read_file(String p_path, bool p_case_sensitive) {
ERR_FAIL_COND_V_MSG(fa.is_null(), PackedByteArray(), "ZIPReader must be opened before use.");
int cs = p_case_sensitive ? 1 : 2;
if (unzLocateFile(uzf, p_path.utf8().get_data(), cs) != UNZ_OK) {
ERR_FAIL_V_MSG(PackedByteArray(), "File does not exist in zip archive: " + p_path);
}
if (unzOpenCurrentFile(uzf) != UNZ_OK) {
ERR_FAIL_V_MSG(PackedByteArray(), "Could not open file within zip archive.");
}
int err = UNZ_OK;
// Locate and open the file.
err = godot_unzip_locate_file(uzf, p_path, p_case_sensitive);
ERR_FAIL_COND_V_MSG(err != UNZ_OK, PackedByteArray(), "File does not exist in zip archive: " + p_path);
err = unzOpenCurrentFile(uzf);
ERR_FAIL_COND_V_MSG(err != UNZ_OK, PackedByteArray(), "Could not open file within zip archive.");
// Read the file info.
unz_file_info info;
unzGetCurrentFileInfo(uzf, &info, NULL, 0, NULL, 0, NULL, 0);
err = unzGetCurrentFileInfo(uzf, &info, nullptr, 0, nullptr, 0, nullptr, 0);
ERR_FAIL_COND_V_MSG(err != UNZ_OK, PackedByteArray(), "Unable to read file information from zip archive.");
ERR_FAIL_COND_V_MSG(info.uncompressed_size > INT_MAX, PackedByteArray(), "File contents too large to read from zip archive (>2 GB).");
// Read the file data.
PackedByteArray data;
data.resize(info.uncompressed_size);
uint8_t *buffer = data.ptrw();
int to_read = data.size();
while (to_read > 0) {
int bytes_read = unzReadCurrentFile(uzf, buffer, to_read);
ERR_FAIL_COND_V_MSG(bytes_read < 0, PackedByteArray(), "IO/zlib error reading file from zip archive.");
ERR_FAIL_COND_V_MSG(bytes_read == UNZ_EOF && to_read != 0, PackedByteArray(), "Incomplete file read from zip archive.");
DEV_ASSERT(bytes_read <= to_read);
buffer += bytes_read;
to_read -= bytes_read;
}
uint8_t *w = data.ptrw();
unzReadCurrentFile(uzf, &w[0], info.uncompressed_size);
unzCloseCurrentFile(uzf);
// Verify the data and return.
err = unzCloseCurrentFile(uzf);
ERR_FAIL_COND_V_MSG(err != UNZ_OK, PackedByteArray(), "CRC error reading file from zip archive.");
return data;
}

View File

@ -40,7 +40,7 @@ class ZIPReader : public RefCounted {
GDCLASS(ZIPReader, RefCounted)
Ref<FileAccess> fa;
unzFile uzf;
unzFile uzf = nullptr;
protected:
static void _bind_methods();