summaryrefslogtreecommitdiff
blob: b5c0608d91ffa8d0b47f8c63d04c05ab71fd293a (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
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;
         }