From 8cf11f9208ff4e351f76b93fa3ab76db10863b24 Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 24 Mar 2020 13:51:39 +0000 Subject: [PATCH] Packages: fix handling of unsafe paths in zips This was breaking the P-51D which contains a mis-named PNG file which tripped the unsafe path detection. --- simgear/io/untar.cxx | 42 ++++++++++++++++++++----------------- simgear/package/Install.cxx | 3 +++ simgear/package/pkgutil.cxx | 8 +++---- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/simgear/io/untar.cxx b/simgear/io/untar.cxx index 4002b61b..61746317 100644 --- a/simgear/io/untar.cxx +++ b/simgear/io/untar.cxx @@ -340,7 +340,7 @@ public: } if (!isSafePath(tarPath)) { - SG_LOG(SG_IO, SG_WARN, "bad tar path:" << tarPath); + SG_LOG(SG_IO, SG_WARN, "unsafe tar path, skipping::" << tarPath); skipCurrentEntry = true; } @@ -506,7 +506,7 @@ public: #endif unzFile zip = unzOpen2(bufferName, &memoryAccessFuncs); - const size_t BUFFER_SIZE = 32 * 1024; + const size_t BUFFER_SIZE = 1024 * 1024; void* buf = malloc(BUFFER_SIZE); try { @@ -532,24 +532,28 @@ public: state = END_OF_ARCHIVE; } catch (sg_exception&) { - state = BAD_ARCHIVE; - } - - free(buf); - unzClose(zip); - } - - void extractCurrentFile(unzFile zip, char* buffer, size_t bufferSize) - { - unz_file_info fileInfo; - unzGetCurrentFileInfo(zip, &fileInfo, - buffer, bufferSize, - NULL, 0, /* extra field */ - NULL, 0 /* comment field */); - + state = BAD_ARCHIVE; + } + + free(buf); + unzClose(zip); + } + + void extractCurrentFile(unzFile zip, char* buffer, size_t bufferSize) + { + unz_file_info fileInfo; + int result = unzGetCurrentFileInfo(zip, &fileInfo, + buffer, bufferSize, + NULL, 0, /* extra field */ + NULL, 0 /* comment field */); + if (result != Z_OK) { + throw sg_io_exception("Failed to get zip current file info"); + } + std::string name(buffer); if (!isSafePath(name)) { - throw sg_format_exception("Bad zip path", name); + SG_LOG(SG_IO, SG_WARN, "unsafe zip path, skipping::" << name); + return; } auto filterResult = filterPath(name); @@ -568,7 +572,7 @@ public: return; } - int result = unzOpenCurrentFile(zip); + result = unzOpenCurrentFile(zip); if (result != UNZ_OK) { throw sg_io_exception("opening current zip file failed", sg_location(name)); } diff --git a/simgear/package/Install.cxx b/simgear/package/Install.cxx index 117d4021..93d09b9e 100644 --- a/simgear/package/Install.cxx +++ b/simgear/package/Install.cxx @@ -122,6 +122,9 @@ protected: m_downloaded += n; m_owner->installProgress(m_downloaded, responseLength()); m_extractor->extractBytes(ubytes, n); + if (m_extractor->hasError()) { + SG_LOG(SG_GENERAL, SG_WARN, "archive extraction failed"); + } } void onDone() override diff --git a/simgear/package/pkgutil.cxx b/simgear/package/pkgutil.cxx index 4fc0dc7a..1641126c 100644 --- a/simgear/package/pkgutil.cxx +++ b/simgear/package/pkgutil.cxx @@ -59,7 +59,7 @@ public: virtual void installProgress(pkg::InstallRef aInstall, unsigned int bytes, unsigned int total) { - unsigned int percent = (bytes * 100) / total; + size_t percent = (static_cast(bytes) * 100) / total; if (percent == _lastPercent) { return; } @@ -78,7 +78,7 @@ public: } private: - unsigned int _lastPercent; + size_t _lastPercent; }; @@ -124,8 +124,8 @@ int main(int argc, char** argv) { HTTP::Client* http = new HTTP::Client(); - pkg::Root* root = new pkg::Root(Dir::current().path(), ""); - + pkg::Root* root = new pkg::Root(Dir::current().path(), "2019.1.1"); + MyDelegate dlg; root->addDelegate(&dlg);