commit 0cb243f64eef45565741b27364cece7d5c349c37 Author: Andreas Cord-Landwehr Date: Tue Jun 14 15:52:49 2016 +0200 Ensure extraction location to be in subfolder Behavior change: Switch to Tar's default behavior to avoid extraction to arbitrary system locations outside of extraction folder. Instead, extract such files to root location in extraction folder. REVIEW: 128185 diff --git a/autotests/karchivetest.cpp b/autotests/karchivetest.cpp index c8abddf..549ed26 100644 --- a/autotests/karchivetest.cpp +++ b/autotests/karchivetest.cpp @@ -760,6 +760,24 @@ void KArchiveTest::testTarDirectoryTwice() // bug 206994 QCOMPARE(listing.count(), 3); } + +void KArchiveTest::testTarIgnoreRelativePathOutsideArchive() +{ + // This test extracts a Tar archive that contains a relative path "../foo" pointing + // outside of the archive directory. For security reasons extractions should only + // be allowed within the extracted directory as long as not specifically asked. + + KTar tar(QFINDTESTDATA(QLatin1String("tar_relative_path_outside_archive.tar.bz2"))); + QVERIFY(tar.open(QIODevice::ReadOnly)); + + const KArchiveDirectory *dir = tar.directory(); + QTemporaryDir tmpDir; + const QString dirName = tmpDir.path() + '/'; + + QVERIFY(dir->copyTo(dirName)); + QVERIFY(!QFile::exists(dirName + "../foo")); + QVERIFY(QFile::exists(dirName + "/foo")); +} /// static const char s_zipFileName[] = "karchivetest.zip"; diff --git a/autotests/karchivetest.h b/autotests/karchivetest.h index 4b7ecff..5a6375c 100644 --- a/autotests/karchivetest.h +++ b/autotests/karchivetest.h @@ -76,6 +76,7 @@ private Q_SLOTS: void testTarDirectoryForgotten(); void testTarRootDir(); void testTarDirectoryTwice(); + void testTarIgnoreRelativePathOutsideArchive(); void testCreateZip(); void testCreateZipError(); diff --git a/autotests/tar_relative_path_outside_archive.tar.bz2 b/autotests/tar_relative_path_outside_archive.tar.bz2 new file mode 100644 index 0000000..50a3aca Binary files /dev/null and b/autotests/tar_relative_path_outside_archive.tar.bz2 differ diff --git a/src/karchive.cpp b/src/karchive.cpp index 5a7cfc6..7683c7f 100644 --- a/src/karchive.cpp +++ b/src/karchive.cpp @@ -841,6 +841,7 @@ static bool sortByPosition(const KArchiveFile *file1, const KArchiveFile *file2) bool KArchiveDirectory::copyTo(const QString &dest, bool recursiveCopy) const { QDir root; + const QString destDir(QDir(dest).absolutePath()); // get directory path without any "." or ".." QList fileList; QMap fileToDir; @@ -850,10 +851,20 @@ bool KArchiveDirectory::copyTo(const QString &dest, bool recursiveCopy) const QStack dirNameStack; dirStack.push(this); // init stack at current directory - dirNameStack.push(dest); // ... with given path + dirNameStack.push(destDir); // ... with given path do { const KArchiveDirectory *curDir = dirStack.pop(); - const QString curDirName = dirNameStack.pop(); + + // extract only to specified folder if it is located within archive's extraction folder + // otherwise put file under root position in extraction folder + QString curDirName = dirNameStack.pop(); + if (!QDir(curDirName).absolutePath().startsWith(destDir)) { + qWarning() << "Attempted export into folder" << curDirName + << "which is outside of the extraction root folder" << destDir << "." + << "Changing export of contained files to extraction root folder."; + curDirName = destDir; + } + if (!root.mkpath(curDirName)) { return false; }