From 6be7eed878bc701407c6c3efd93d9944e1079490 Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Fri, 10 Feb 2023 09:17:48 +0100 Subject: [PATCH 1/2] fix: don't extract files outside of target path This should fix a security issue regarding path traversal in zip files. Signed-off-by: Sefa Eyeoglu --- launcher/MMCZip.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/launcher/MMCZip.cpp b/launcher/MMCZip.cpp index f6600343..734eacd8 100644 --- a/launcher/MMCZip.cpp +++ b/launcher/MMCZip.cpp @@ -275,7 +275,8 @@ bool MMCZip::findFilesInZip(QuaZip * zip, const QString & what, QStringList & re // ours std::optional MMCZip::extractSubDir(QuaZip *zip, const QString & subdir, const QString &target) { - QDir directory(target); + auto absDirectoryUrl = QUrl::fromLocalFile(target); + QStringList extracted; qDebug() << "Extracting subdir" << subdir << "from" << zip->getZipName() << "to" << target; @@ -317,11 +318,16 @@ std::optional MMCZip::extractSubDir(QuaZip *zip, const QString & su QString absFilePath; if(name.isEmpty()) { - absFilePath = directory.absoluteFilePath(name) + "/"; + absFilePath = FS::PathCombine(target, "/"); // FIXME this seems weird } else { - absFilePath = directory.absoluteFilePath(path + name); + absFilePath = FS::PathCombine(target, path + name); + } + + if (!absDirectoryUrl.isParentOf(QUrl::fromLocalFile(absFilePath))) { + qWarning() << "Extracting" << name << "was cancelled, because it was effectively outside of the target path" << target; + return std::nullopt; } if (!JlCompress::extractFile(zip, "", absFilePath)) From e70a5a47ee8bf67715a46a6ac668dad7685d08be Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Fri, 10 Feb 2023 10:46:21 +0100 Subject: [PATCH 2/2] fix: ignore absolute paths when extracting Signed-off-by: Sefa Eyeoglu --- launcher/MMCZip.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/launcher/MMCZip.cpp b/launcher/MMCZip.cpp index 734eacd8..31460bf4 100644 --- a/launcher/MMCZip.cpp +++ b/launcher/MMCZip.cpp @@ -306,6 +306,11 @@ std::optional MMCZip::extractSubDir(QuaZip *zip, const QString & su name.remove(0, subdir.size()); auto original_name = name; + // Fix subdirs/files ending with a / getting transformed into absolute paths + if(name.startsWith('/')){ + name = name.mid(1); + } + // Fix weird "folders with a single file get squashed" thing QString path; if(name.contains('/') && !name.endsWith('/')){