CVE-2017-6200: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: Sandstorm before build 0.203 allows remote attackers to read any specified file under /etc or /run via the sandbox backup function. The root cause is that the findFilesToZip function doesn't filter Line Feed (\n) characters in a directory name.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 6.5
Published: 2018-02-06T16:29Z
Repository: https://github.com/sandstorm-io/sandstorm
Language: Other
Files changed: src/sandstorm/backup.c++
Commit date: 2017-03-01 14:23:15 -0800
Commit message: Fix newline detection in zip paths.

Diff: diff --git a/src/sandstorm/backup.c++ b/src/sandstorm/backup.c++
index 16d3cf455..20349e61c 100644
--- a/src/sandstorm/backup.c++
+++ b/src/sandstorm/backup.c++
@@ -250,27 +250,27 @@ void BackupMain::pump(kj::InputStream& in, kj::OutputStream& out) {
     out.write(buffer, n);
   }
 }
 
 bool BackupMain::findFilesToZip(kj::StringPtr path, kj::OutputStream& out) {
+  // If the path contains a newline, we cannot correctly pass it to `zip` since `zip` expects
+  // one file per line. For security reasons, we must detect and filter out these files.
+  // Hopefully this never happens legitimately?
+  if (path.findFirst('\n') != nullptr) {
+    KJ_LOG(ERROR, "tried to backup file containing newlines", path);
+    return false;
+  }
+
   struct stat stats;
   KJ_SYSCALL(lstat(path.cStr(), &stats));
   if (S_ISREG(stats.st_mode) || S_ISLNK(stats.st_mode)) {
     // Regular file or link can be zipped; write to file stream.
-    // If the path contains a newline, we cannot correctly pass it to `zip` since `zip` expects
-    // one file per line. For security reasons, we must detect and filter out these files.
-    // Hopefully this never happens legitimately?
-    if (path.findFirst('\n') == nullptr) {
-      kj::ArrayPtr<const byte> pieces[2];
-      pieces[0] = path.asBytes();
-      pieces[1] = kj::StringPtr("\n").asBytes();
-      out.write(pieces);
-      return true;
-    } else {
-      KJ_LOG(ERROR, "tried to backup file containing newlines", path);
-      return false;
-    }
+    kj::ArrayPtr<const byte> pieces[2];
+    pieces[0] = path.asBytes();
+    pieces[1] = kj::StringPtr("\n").asBytes();
+    out.write(pieces);
+    return true;
   } else if (S_ISDIR(stats.st_mode)) {
     // Subdirectory; enumerate contents.
     bool packedAny = false;
     for (auto& entry: listDirectory(path)) {
       if (findFilesToZip(kj::str(path, '/', entry), out)) {

Vulnerable code:     // If the path contains a newline, we cannot correctly pass it to `zip` since `zip` expects
    // one file per line. For security reasons, we must detect and filter out these files.
    // Hopefully this never happens legitimately?
    if (path.findFirst('\n') == nullptr) {
      kj::ArrayPtr<const byte> pieces[2];
      pieces[0] = path.asBytes();
      pieces[1] = kj::StringPtr("\n").asBytes();
      out.write(pieces);
      return true;
    } else {
      KJ_LOG(ERROR, "tried to backup file containing newlines", path);
      return false;
    }
Fixed code:   // If the path contains a newline, we cannot correctly pass it to `zip` since `zip` expects
  // one file per line. For security reasons, we must detect and filter out these files.
  // Hopefully this never happens legitimately?
  if (path.findFirst('\n') != nullptr) {
    KJ_LOG(ERROR, "tried to backup file containing newlines", path);
    return false;
  }

    kj::ArrayPtr<const byte> pieces[2];
    pieces[0] = path.asBytes();
    pieces[1] = kj::StringPtr("\n").asBytes();
    out.write(pieces);
    return true;