widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1738553-zip-filesystem-overflow into lp:widelands.

Commit message:
Corrupt zip files now appear as "incompatible" on game loading/saving screens. This fixes 
stack-buffer-overflows.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1738553 in widelands: "stack-buffer-overflow in zip filesystem"
  https://bugs.launchpad.net/widelands/+bug/1738553

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1738553-zip-filesystem-overflow/+merge/339493
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1738553-zip-filesystem-overflow into lp:widelands.

=== modified file 'src/io/filesystem/zip_filesystem.cc'
--- src/io/filesystem/zip_filesystem.cc	2017-11-09 17:37:13 +0000
+++ src/io/filesystem/zip_filesystem.cc	2018-02-25 11:31:03 +0000
@@ -217,9 +217,14 @@
 	assert(path_in.size());
 
 	for (;;) {
-		unzGetCurrentFileInfo(zip_file_->read_handle(), &file_info, filename_inzip,
+		const int32_t success = unzGetCurrentFileInfo(zip_file_->read_handle(), &file_info, filename_inzip,
 		                      sizeof(filename_inzip), nullptr, 0, nullptr, 0);
 
+		// Handle corrupt files
+		if (success != UNZ_OK) {
+			return false;
+		}
+
 		std::string complete_filename = zip_file_->strip_basename(filename_inzip);
 
 		if (*complete_filename.rbegin() == '/')
@@ -359,9 +364,14 @@
  * \throw FileNotFoundError if the file couldn't be opened.
  */
 void* ZipFilesystem::load(const std::string& fname, size_t& length) {
-	if (!file_exists(fname.c_str()) || is_directory(fname.c_str()))
+	try {
+		if (!file_exists(fname.c_str()) || is_directory(fname.c_str()))
+			throw ZipOperationError(
+				"ZipFilesystem::load", fname, zip_file_->path(), "could not open file from zipfile");
+	}  catch (const ZipOperationError& e) {
 		throw ZipOperationError(
-		   "ZipFilesystem::load", fname, zip_file_->path(), "could not open file from zipfile");
+			"ZipFilesystem::load", fname, zip_file_->path(), e.what());
+	}
 
 	char buffer[1024];
 	size_t totallen = 0;