From 6be7eed878bc701407c6c3efd93d9944e1079490 Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Fri, 10 Feb 2023 09:17:48 +0100 Subject: [PATCH] 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 f66003432..734eacd84 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))