CVE-2018-1002209: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: QuaZIP before 0.7.6 is vulnerable to directory traversal, allowing attackers to write to arbitrary files via a ../ (dot dot slash) in a Zip archive entry that is mishandled during extraction. This vulnerability is also known as 'Zip-Slip'.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 5.5
Published: 2018-07-25T17:29Z
Repository: https://github.com/stachenov/quazip
Language: C++
Files changed: NEWS.txt, quazip/JlCompress.cpp, qztest/testjlcompress.cpp
Commit date: 2018-06-12 21:32:45 +0300
Commit message: Fixed the Zip Slip vulnerability in JlCompress

When extracting a file with a dangerous path like "../evil.exe"
from a ZIP archive with JlCompress::extractDir(), the target
file would be created outside of the target directory, potentially
even overwriting an existing file there.

Diff: diff --git a/NEWS.txt b/NEWS.txt
index f5c4007..5528ecb 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -1,7 +1,10 @@
 QuaZIP changes
 
+* Current
+        * Fixed the Zip Slip vulnerability in JlCompress
+
 * 0.7.5
         * Fixed target_link_libraries call in CMakeLists
         * Worked around a Qt 4.6 bug (QTBUG-15421) screwing up hidden
           files handling in JlCompress::compressDir()
         * Removed Q_FOREACH uses to avoid conflicts (SF patch #32)
diff --git a/quazip/JlCompress.cpp b/quazip/JlCompress.cpp
index 73464d2..40978b1 100644
--- a/quazip/JlCompress.cpp
+++ b/quazip/JlCompress.cpp
@@ -362,19 +362,23 @@ QStringList JlCompress::extractDir(QString fileCompressed, QString dir) {
 QStringList JlCompress::extractDir(QuaZip &zip, const QString &dir)
 {
     if(!zip.open(QuaZip::mdUnzip)) {
         return QStringList();
     }
-
-    QDir directory(dir);
+    QString cleanDir = QDir::cleanPath(dir);
+    QDir directory(cleanDir);
+    QString absCleanDir = directory.absolutePath();
     QStringList extracted;
     if (!zip.goToFirstFile()) {
         return QStringList();
     }
     do {
         QString name = zip.getCurrentFileName();
         QString absFilePath = directory.absoluteFilePath(name);
+        QString absCleanPath = QDir::cleanPath(absFilePath);
+        if (!absCleanPath.startsWith(absCleanDir + "/"))
+            continue;
         if (!extractFile(&zip, "", absFilePath)) {
             removeFile(extracted);
             return QStringList();
         }
         extracted.append(absFilePath);
diff --git a/qztest/testjlcompress.cpp b/qztest/testjlcompress.cpp
index 25c763b..ad1cd0a 100644
--- a/qztest/testjlcompress.cpp
+++ b/qztest/testjlcompress.cpp
@@ -300,21 +300,29 @@ void TestJlCompress::extractFiles()
 
 void TestJlCompress::extractDir_data()
 {
     QTest::addColumn<QString>("zipName");
     QTest::addColumn<QStringList>("fileNames");
-    QTest::newRow("simple") << "jlextdir.zip" << (
-            QStringList() << "test0.txt" << "testdir1/test1.txt"
+    QTest::addColumn<QStringList>("expectedExtracted");
+    QTest::newRow("simple") << "jlextdir.zip"
+        << (QStringList() << "test0.txt" << "testdir1/test1.txt"
+            << "testdir2/test2.txt" << "testdir2/subdir/test2sub.txt")
+        << (QStringList() << "test0.txt" << "testdir1/test1.txt"
             << "testdir2/test2.txt" << "testdir2/subdir/test2sub.txt");
-    QTest::newRow("separate dir") << "sepdir.zip" << (
-            QStringList() << "laj/" << "laj/lajfile.txt");
+    QTest::newRow("separate dir") << "sepdir.zip"
+        << (QStringList() << "laj/" << "laj/lajfile.txt")
+        << (QStringList() << "laj/" << "laj/lajfile.txt");
+    QTest::newRow("Zip Slip") << "zipslip.zip"
+        << (QStringList() << "test0.txt" << "../zipslip.txt")
+        << (QStringList() << "test0.txt");
 }
 
 void TestJlCompress::extractDir()
 {
     QFETCH(QString, zipName);
     QFETCH(QStringList, fileNames);
+    QFETCH(QStringList, expectedExtracted);
     QDir curDir;
     if (!curDir.mkpath("jlext/jldir")) {
         QFAIL("Couldn't mkpath jlext/jldir");
     }
     if (!createTestFiles(fileNames)) {
@@ -323,40 +331,41 @@ void TestJlCompress::extractDir()
     if (!createTestArchive(zipName, fileNames)) {
         QFAIL("Couldn't create test archive");
     }
     QStringList extracted;
     QCOMPARE((extracted = JlCompress::extractDir(zipName, "jlext/jldir"))
-        .count(), fileNames.count());
-    foreach (QString fileName, fileNames) {
-        QString fullName = "jlext/jldir/" + fileName;
+        .count(), expectedExtracted.count());
+    const QString dir = "jlext/jldir/";
+    foreach (QString fileName, expectedExtracted) {
+        QString fullName = dir + fileName;
         QFileInfo fileInfo(fullName);
         QFileInfo extInfo("tmp/" + fileName);
         if (!fileInfo.isDir())
             QCOMPARE(fileInfo.size(), extInfo.size());
         QCOMPARE(fileInfo.permissions(), extInfo.permissions());
         curDir.remove(fullName);
         curDir.rmpath(fileInfo.dir().path());
-        QString absolutePath = fileInfo.absoluteFilePath();
+        QString absolutePath = QDir(dir).absoluteFilePath(fileName);
         if (fileInfo.isDir() && !absolutePath.endsWith('/'))
 	    absolutePath += '/';
         QVERIFY(extracted.contains(absolutePath));
     }
     // now test the QIODevice* overload
     QFile zipFile(zipName);
     QVERIFY(zipFile.open(QIODevice::ReadOnly));
     QCOMPARE((extracted = JlCompress::extractDir(&zipFile, "jlext/jldir"))
-        .count(), fileNames.count());
-    foreach (QString fileName, fileNames) {
-        QString fullName = "jlext/jldir/" + fileName;
+        .count(), expectedExtracted.count());
+    foreach (QString fileName, expectedExtracted) {
+        QString fullName = dir + fileName;
         QFileInfo fileInfo(fullName);
         QFileInfo extInfo("tmp/" + fileName);
         if (!fileInfo.isDir())
             QCOMPARE(fileInfo.size(), extInfo.size());
         QCOMPARE(fileInfo.permissions(), extInfo.permissions());
         curDir.remove(fullName);
         curDir.rmpath(fileInfo.dir().path());
-        QString absolutePath = fileInfo.absoluteFilePath();
+        QString absolutePath = QDir(dir).absoluteFilePath(fileName);
         if (fileInfo.isDir() && !absolutePath.endsWith('/'))
         absolutePath += '/';
         QVERIFY(extracted.contains(absolutePath));
     }
     zipFile.close();

Vulnerable code: 
    QDir directory(dir);
    QTest::newRow("simple") << "jlextdir.zip" << (
            QStringList() << "test0.txt" << "testdir1/test1.txt"
    QTest::newRow("separate dir") << "sepdir.zip" << (
            QStringList() << "laj/" << "laj/lajfile.txt");
        .count(), fileNames.count());
    foreach (QString fileName, fileNames) {
        QString fullName = "jlext/jldir/" + fileName;
        QString absolutePath = fileInfo.absoluteFilePath();
        .count(), fileNames.count());
    foreach (QString fileName, fileNames) {
        QString fullName = "jlext/jldir/" + fileName;
        QString absolutePath = fileInfo.absoluteFilePath();
Fixed code: * Current
        * Fixed the Zip Slip vulnerability in JlCompress

    QString cleanDir = QDir::cleanPath(dir);
    QDir directory(cleanDir);
    QString absCleanDir = directory.absolutePath();
        QString absCleanPath = QDir::cleanPath(absFilePath);
        if (!absCleanPath.startsWith(absCleanDir + "/"))
            continue;
    QTest::addColumn<QStringList>("expectedExtracted");
    QTest::newRow("simple") << "jlextdir.zip"
        << (QStringList() << "test0.txt" << "testdir1/test1.txt"
            << "testdir2/test2.txt" << "testdir2/subdir/test2sub.txt")
        << (QStringList() << "test0.txt" << "testdir1/test1.txt"
    QTest::newRow("separate dir") << "sepdir.zip"
        << (QStringList() << "laj/" << "laj/lajfile.txt")
        << (QStringList() << "laj/" << "laj/lajfile.txt");
    QTest::newRow("Zip Slip") << "zipslip.zip"
        << (QStringList() << "test0.txt" << "../zipslip.txt")
        << (QStringList() << "test0.txt");
    QFETCH(QStringList, expectedExtracted);
        .count(), expectedExtracted.count());
    const QString dir = "jlext/jldir/";
    foreach (QString fileName, expectedExtracted) {
        QString fullName = dir + fileName;
        QString absolutePath = QDir(dir).absoluteFilePath(fileName);
        .count(), expectedExtracted.count());
    foreach (QString fileName, expectedExtracted) {
        QString fullName = dir + fileName;
        QString absolutePath = QDir(dir).absoluteFilePath(fileName);