CVE-2020-15908: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: tar/TarFileReader.cpp in Cauldron cbang (aka C-Bang or C!) before 1.6.0 allows Directory Traversal during extraction from a TAR archive.
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: 7.5
Published: 2020-07-23T04:15Z
Repository: https://github.com/CauldronDevelopmentLLC/cbang
Language: Other
Files changed: src/cbang/tar/TarFileReader.cpp, tests/tarTests/.gitignore, tests/tarTests/BadExtractTest/data/args, tests/tarTests/BadExtractTest/expect/return, tests/tarTests/BadExtractTest/expect/stderr, tests/tarTests/CompressedExtractTest/data/args, tests/tarTests/CompressedExtractTest/expect/return, tests/tarTests/CompressedExtractTest/expect/stdout, tests/tarTests/ExtractTest/data/args, tests/tarTests/ExtractTest/expect/return, tests/tarTests/ExtractTest/expect/stdout, tests/tarTests/SConscript, tests/tarTests/tar.cpp, tests/tarTests/test.json
Commit date: 2020-07-19 12:43:49 -0700
Commit message: Don't allow extraction of tar files outside of the target directory, added tar tests

Diff: diff --git a/src/cbang/tar/TarFileReader.cpp b/src/cbang/tar/TarFileReader.cpp
index 6504cd56..6621642f 100644
--- a/src/cbang/tar/TarFileReader.cpp
+++ b/src/cbang/tar/TarFileReader.cpp
@@ -99,15 +99,30 @@ bool TarFileReader::next() {
 std::string TarFileReader::extract(const string &_path) {
   if (_path.empty()) THROW("path cannot be empty");
   if (!hasMore()) THROW("No more tar files");
 
   string path = _path;
-  if (SystemUtilities::isDirectory(path)) path += "/" + getFilename();
+  if (SystemUtilities::isDirectory(path)) {
+    path += "/" + getFilename();
+
+    // Check that path is under the target directory
+    string a = SystemUtilities::getCanonicalPath(_path);
+    string b = SystemUtilities::getCanonicalPath(path);
+    if (!String::startsWith(b, a))
+      THROW("Tar path points outside of the extraction directory: " << path);
+  }
 
   LOG_DEBUG(5, "Extracting: " << path);
 
-  return extract(*SystemUtilities::oopen(path));
+  switch (getType()) {
+  case NORMAL_FILE: case CONTIGUOUS_FILE:
+    return extract(*SystemUtilities::oopen(path));
+  case DIRECTORY: SystemUtilities::ensureDirectory(path); break;
+  default: THROW("Unsupported tar file type " << getType());
+  }
+
+  return getFilename();
 }
 
 
 string TarFileReader::extract(ostream &out) {
   if (!hasMore()) THROW("No more tar files");
diff --git a/tests/tarTests/.gitignore b/tests/tarTests/.gitignore
new file mode 100644
index 00000000..e7c01fbe
--- /dev/null
+++ b/tests/tarTests/.gitignore
@@ -0,0 +1 @@
+/path
diff --git a/tests/tarTests/BadExtractTest/data/args b/tests/tarTests/BadExtractTest/data/args
new file mode 100644
index 00000000..3275cb0c
--- /dev/null
+++ b/tests/tarTests/BadExtractTest/data/args
@@ -0,0 +1 @@
+--extract test.tar
diff --git a/tests/tarTests/BadExtractTest/data/test.tar b/tests/tarTests/BadExtractTest/data/test.tar
new file mode 100644
index 00000000..d8e1ecd5
Binary files /dev/null and b/tests/tarTests/BadExtractTest/data/test.tar differ
diff --git a/tests/tarTests/BadExtractTest/expect/return b/tests/tarTests/BadExtractTest/expect/return
new file mode 100644
index 00000000..d00491fd
--- /dev/null
+++ b/tests/tarTests/BadExtractTest/expect/return
@@ -0,0 +1 @@
+1
diff --git a/tests/tarTests/BadExtractTest/expect/stderr b/tests/tarTests/BadExtractTest/expect/stderr
new file mode 100644
index 00000000..8ab2216a
--- /dev/null
+++ b/tests/tarTests/BadExtractTest/expect/stderr
@@ -0,0 +1 @@
+Tar path points outside of the extraction directory: ./../hello.txt
\ No newline at end of file
diff --git a/tests/tarTests/BadExtractTest/expect/stdout b/tests/tarTests/BadExtractTest/expect/stdout
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/tarTests/CompressedExtractTest/data/args b/tests/tarTests/CompressedExtractTest/data/args
new file mode 100644
index 00000000..ef6d4052
--- /dev/null
+++ b/tests/tarTests/CompressedExtractTest/data/args
@@ -0,0 +1 @@
+--extract test.tar.bz2
diff --git a/tests/tarTests/CompressedExtractTest/data/test.tar.bz2 b/tests/tarTests/CompressedExtractTest/data/test.tar.bz2
new file mode 100644
index 00000000..0a07aa37
Binary files /dev/null and b/tests/tarTests/CompressedExtractTest/data/test.tar.bz2 differ
diff --git a/tests/tarTests/CompressedExtractTest/expect/return b/tests/tarTests/CompressedExtractTest/expect/return
new file mode 100644
index 00000000..573541ac
--- /dev/null
+++ b/tests/tarTests/CompressedExtractTest/expect/return
@@ -0,0 +1 @@
+0
diff --git a/tests/tarTests/CompressedExtractTest/expect/stderr b/tests/tarTests/CompressedExtractTest/expect/stderr
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/tarTests/CompressedExtractTest/expect/stdout b/tests/tarTests/CompressedExtractTest/expect/stdout
new file mode 100644
index 00000000..f0ad0dec
--- /dev/null
+++ b/tests/tarTests/CompressedExtractTest/expect/stdout
@@ -0,0 +1 @@
+hello.txt
diff --git a/tests/tarTests/ExtractTest/data/args b/tests/tarTests/ExtractTest/data/args
new file mode 100644
index 00000000..3275cb0c
--- /dev/null
+++ b/tests/tarTests/ExtractTest/data/args
@@ -0,0 +1 @@
+--extract test.tar
diff --git a/tests/tarTests/ExtractTest/data/test.tar b/tests/tarTests/ExtractTest/data/test.tar
new file mode 100644
index 00000000..56cdac64
Binary files /dev/null and b/tests/tarTests/ExtractTest/data/test.tar differ
diff --git a/tests/tarTests/ExtractTest/expect/return b/tests/tarTests/ExtractTest/expect/return
new file mode 100644
index 00000000..573541ac
--- /dev/null
+++ b/tests/tarTests/ExtractTest/expect/return
@@ -0,0 +1 @@
+0
diff --git a/tests/tarTests/ExtractTest/expect/stderr b/tests/tarTests/ExtractTest/expect/stderr
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/tarTests/ExtractTest/expect/stdout b/tests/tarTests/ExtractTest/expect/stdout
new file mode 100644
index 00000000..f0ad0dec
--- /dev/null
+++ b/tests/tarTests/ExtractTest/expect/stdout
@@ -0,0 +1 @@
+hello.txt
diff --git a/tests/tarTests/SConscript b/tests/tarTests/SConscript
new file mode 100644
index 00000000..7915cbcd
--- /dev/null
+++ b/tests/tarTests/SConscript
@@ -0,0 +1,8 @@
+Import('*')
+
+# Local includes
+env.Append(CPPPATH = ['#'])
+
+prog = env.Program('tar', 'tar.cpp')
+
+Return('prog')
diff --git a/tests/tarTests/tar b/tests/tarTests/tar
new file mode 100755
index 00000000..0fc9a64f
Binary files /dev/null and b/tests/tarTests/tar differ
diff --git a/tests/tarTests/tar.cpp b/tests/tarTests/tar.cpp
new file mode 100644
index 00000000..f2b4a0df
--- /dev/null
+++ b/tests/tarTests/tar.cpp
@@ -0,0 +1,61 @@
+/******************************************************************************\
+
+          This file is part of the C! library.  A.K.A the cbang library.
+
+                Copyright (c) 2003-2019, Cauldron Development LLC
+                   Copyright (c) 2003-2017, Stanford University
+                               All rights reserved.
+
+         The C! library is free software: you can redistribute it and/or
+        modify it under the terms of the GNU Lesser General Public License
+       as published by the Free Software Foundation, either version 2.1 of
+               the License, or (at your option) any later version.
+
+        The C! library is distributed in the hope that it will be useful,
+          but WITHOUT ANY WARRANTY; without even the implied warranty of
+        MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+                 Lesser General Public License for more details.
+
+         You should have received a copy of the GNU Lesser General Public
+                 License along with the C! library.  If not, see
+                         <http://www.gnu.org/licenses/>.
+
+        In addition, BSD licensing may be granted on a case by case basis
+        by written permission from at least one of the copyright holders.
+           You may request written permission by emailing the authors.
+
+                  For information regarding this software email:
+                                 Joseph Coffland
+                          joseph@cauldrondevelopment.com
+
+\******************************************************************************/
+
+#include <cbang/tar/TarFileReader.h>
+#include <cbang/Catch.h>
+
+#include <iostream>
+
+using namespace cb;
+using namespace std;
+
+
+int main(int argc, char *argv[]) {
+  try {
+    for (int i = 1; i < argc; i++) {
+      string arg = argv[i];
+
+      if (arg == "--extract" && i < argc - 1) {
+        TarFileReader reader(argv[++i]);
+
+        while (reader.hasMore())
+          cout << reader.extract() << endl;
+
+      } else THROWS("Invalid arg '" << arg << "'");
+    }
+
+    return 0;
+
+  } catch (const Exception &e) {cerr << e.getMessage();}
+
+  return 1;
+}
diff --git a/tests/tarTests/test.json b/tests/tarTests/test.json
new file mode 100644
index 00000000..a3dd7186
--- /dev/null
+++ b/tests/tarTests/test.json
@@ -0,0 +1,3 @@
+{
+  "command": "%(suite-dir)s/tar"
+}

Vulnerable code:   if (SystemUtilities::isDirectory(path)) path += "/" + getFilename();
  return extract(*SystemUtilities::oopen(path));
Fixed code:   if (SystemUtilities::isDirectory(path)) {
    path += "/" + getFilename();

    // Check that path is under the target directory
    string a = SystemUtilities::getCanonicalPath(_path);
    string b = SystemUtilities::getCanonicalPath(path);
    if (!String::startsWith(b, a))
      THROW("Tar path points outside of the extraction directory: " << path);
  }
  switch (getType()) {
  case NORMAL_FILE: case CONTIGUOUS_FILE:
    return extract(*SystemUtilities::oopen(path));
  case DIRECTORY: SystemUtilities::ensureDirectory(path); break;
  default: THROW("Unsupported tar file type " << getType());
  }

  return getFilename();
/path
--extract test.tar
1
Tar path points outside of the extraction directory: ./../hello.txt
--extract test.tar.bz2
0
hello.txt
--extract test.tar
0
hello.txt
Import('*')

# Local includes
env.Append(CPPPATH = ['#'])

prog = env.Program('tar', 'tar.cpp')

Return('prog')
/******************************************************************************\

          This file is part of the C! library.  A.K.A the cbang library.

                Copyright (c) 2003-2019, Cauldron Development LLC
                   Copyright (c) 2003-2017, Stanford University
                               All rights reserved.

         The C! library is free software: you can redistribute it and/or
        modify it under the terms of the GNU Lesser General Public License
       as published by the Free Software Foundation, either version 2.1 of
               the License, or (at your option) any later version.

        The C! library is distributed in the hope that it will be useful,
          but WITHOUT ANY WARRANTY; without even the implied warranty of
        MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
                 Lesser General Public License for more details.

         You should have received a copy of the GNU Lesser General Public
                 License along with the C! library.  If not, see
                         <http://www.gnu.org/licenses/>.

        In addition, BSD licensing may be granted on a case by case basis
        by written permission from at least one of the copyright holders.
           You may request written permission by emailing the authors.

                  For information regarding this software email:
                                 Joseph Coffland
                          joseph@cauldrondevelopment.com

\******************************************************************************/

#include <cbang/tar/TarFileReader.h>
#include <cbang/Catch.h>

#include <iostream>

using namespace cb;
using namespace std;


int main(int argc, char *argv[]) {
  try {
    for (int i = 1; i < argc; i++) {
      string arg = argv[i];

      if (arg == "--extract" && i < argc - 1) {
        TarFileReader reader(argv[++i]);

        while (reader.hasMore())
          cout << reader.extract() << endl;

      } else THROWS("Invalid arg '" << arg << "'");
    }

    return 0;

  } catch (const Exception &e) {cerr << e.getMessage();}

  return 1;
}
{
  "command": "%(suite-dir)s/tar"
}