blob: b5c0608d91ffa8d0b47f8c63d04c05ab71fd293a [file] [log] [blame]
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;
}