| commit 0cb243f64eef45565741b27364cece7d5c349c37 |
| Author: Andreas Cord-Landwehr <cordlandwehr@kde.org> |
| 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<const KArchiveFile *> fileList; |
| QMap<qint64, QString> fileToDir; |
| @@ -850,10 +851,20 @@ bool KArchiveDirectory::copyTo(const QString &dest, bool recursiveCopy) const |
| QStack<QString> 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; |
| } |