From ea9774f689a36e967c481c56999075671d2463b7 Mon Sep 17 00:00:00 2001 From: Littleboy Date: Sun, 24 Apr 2011 22:45:36 -0400 Subject: GUI: Add basic validity check to ThemeEngine::themeConfigParseHeader (workaround for #3103051) When loading a corrupted zip, the data returned for THEMERC will be garbage, which will cause an assert in isspace() when trying to trim the data. This checks that the first character of the header is in the range [0;127] and bails out if not. --- gui/ThemeEngine.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gui/ThemeEngine.cpp b/gui/ThemeEngine.cpp index 82104eb7ae..2f9c7ae279 100644 --- a/gui/ThemeEngine.cpp +++ b/gui/ThemeEngine.cpp @@ -1502,6 +1502,12 @@ Common::String ThemeEngine::genLocalizedFontFilename(const Common::String &filen *********************************************************/ bool ThemeEngine::themeConfigParseHeader(Common::String header, Common::String &themeName) { + // Check that header is not corrupted + if (header[0] < 0 || header[0] > 127) { + warning("Corrupted theme header found"); + return false; + } + header.trim(); if (header.empty()) -- cgit v1.2.3 From 55650f364cdf9d0b0ff36479dba0e599004e10ca Mon Sep 17 00:00:00 2001 From: Littleboy Date: Sun, 24 Apr 2011 23:53:24 -0400 Subject: COMMON: Add proper error handling to the ZipArchive class - Add check in listMembers and skip files that can't be enumerated. - Add checks for all function calls in createReadStreamForMember (and no longer return a stream from an uninitialized buffer when the file cannot be read). --- common/unzip.cpp | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/common/unzip.cpp b/common/unzip.cpp index cd5d37f4bd..7b78da0faf 100644 --- a/common/unzip.cpp +++ b/common/unzip.cpp @@ -1470,11 +1470,13 @@ int ZipArchive::listMembers(Common::ArchiveMemberList &list) { while (err == UNZ_OK) { char szCurrentFileName[UNZ_MAXFILENAMEINZIP+1]; - unzGetCurrentFileInfo(_zipFile, NULL, - szCurrentFileName, sizeof(szCurrentFileName)-1, - NULL, 0, NULL, 0); - list.push_back(ArchiveMemberList::value_type(new GenericArchiveMember(szCurrentFileName, this))); - matches++; + if (unzGetCurrentFileInfo(_zipFile, NULL, + szCurrentFileName, sizeof(szCurrentFileName)-1, + NULL, 0, NULL, 0) == UNZ_OK) { + list.push_back(ArchiveMemberList::value_type(new GenericArchiveMember(szCurrentFileName, this))); + matches++; + } + err = unzGoToNextFile(_zipFile); } @@ -1493,18 +1495,31 @@ Common::SeekableReadStream *ZipArchive::createReadStreamForMember(const Common:: return 0; unz_file_info fileInfo; - unzOpenCurrentFile(_zipFile); - unzGetCurrentFileInfo(_zipFile, &fileInfo, NULL, 0, NULL, 0, NULL, 0); + if (unzOpenCurrentFile(_zipFile) != UNZ_OK) + return 0; + + if (unzGetCurrentFileInfo(_zipFile, &fileInfo, NULL, 0, NULL, 0, NULL, 0) != UNZ_OK) + return 0; + byte *buffer = (byte *)malloc(fileInfo.uncompressed_size); assert(buffer); - unzReadCurrentFile(_zipFile, buffer, fileInfo.uncompressed_size); - unzCloseCurrentFile(_zipFile); + + if (unzReadCurrentFile(_zipFile, buffer, fileInfo.uncompressed_size) != (int)fileInfo.uncompressed_size) { + free(buffer); + return 0; + } + + if (unzCloseCurrentFile(_zipFile) != UNZ_OK) { + free(buffer); + return 0; + } + return new Common::MemoryReadStream(buffer, fileInfo.uncompressed_size, DisposeAfterUse::YES); // FIXME: instead of reading all into a memory stream, we could // instead create a new ZipStream class. But then we have to be // careful to handle the case where the client code opens multiple - // files in the archive and tries to use them indepenendtly. + // files in the archive and tries to use them independently. } Archive *makeZipArchive(const String &name) { -- cgit v1.2.3