Bug 1289718 - Enforce absolute paths for file access. draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Fri, 19 Aug 2016 18:41:58 +0200
changeset 409931 bd265df759b4d7e3522090697a682f7530bfaaa8
parent 409930 4d761557e46026e3a3ffb4d88be0304b2e9e52c2
child 409932 99dd3a730013fc0aa97be7792cd9e94ea3ea85d3
push id28613
push usergpascutto@mozilla.com
push dateMon, 05 Sep 2016 18:00:21 +0000
bugs1289718
milestone51.0a1
Bug 1289718 - Enforce absolute paths for file access. MozReview-Commit-ID: DCQ9DGBBjm4
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/broker/SandboxBroker.h
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -345,16 +345,33 @@ static int
 DoStat(const char* aPath, struct stat* aStat, int aFlags)
 {
   if (aFlags & O_NOFOLLOW) {
     return lstat(aPath, aStat);
   }
   return stat(aPath, aStat);
 }
 
+size_t
+SandboxBroker::ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen)
+{
+  if (strstr(aPath, "..") != NULL) {
+    char* result = realpath(aPath, NULL);
+    if (result != NULL) {
+      strncpy(aPath, result, aBufSize);
+      aPath[aBufSize - 1] = '\0';
+      free(result);
+      // Size changed, but guaranteed to be 0 terminated
+      aPathLen = strlen(aPath);
+    }
+    // ValidatePath will handle failure to translate
+  }
+  return aPathLen;
+}
+
 void
 SandboxBroker::ThreadMain(void)
 {
   char threadName[16];
   snprintf(threadName, sizeof(threadName), "FS Broker %d", mChildPid);
   PlatformThread::SetName(threadName);
 
   // Permissive mode can only be enabled through an environment variable,
@@ -429,20 +446,18 @@ SandboxBroker::ThreadMain(void)
     int openedFd = -1;
 
     // Look up the pathname.
     pathLen = recvd - sizeof(req);
     // It shouldn't be possible for recvmsg to violate this assertion,
     // but one more predictable branch shouldn't have much perf impact:
     MOZ_RELEASE_ASSERT(pathLen <= kMaxPathLen);
     pathBuf[pathLen] = '\0';
-    int perms = 0;
-    if (!memchr(pathBuf, '\0', pathLen)) {
-      perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
-    }
+    pathLen = ConvertToRealPath(pathBuf, sizeof(pathBuf), pathLen);
+    int perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
 
     // And now perform the operation if allowed.
     if (perms & CRASH_INSTEAD) {
       // This is somewhat nonmodular, but it works.
       resp.mError = ENOSYS;
     } else if (permissive || perms & MAY_ACCESS) {
       // If the operation was only allowed because of permissive mode, log it.
       if (permissive && !(perms & MAY_ACCESS)) {
--- a/security/sandbox/linux/broker/SandboxBroker.h
+++ b/security/sandbox/linux/broker/SandboxBroker.h
@@ -113,16 +113,18 @@ class SandboxBroker final
   int mFileDesc;
   const int mChildPid;
   const UniquePtr<const Policy> mPolicy;
 
   SandboxBroker(UniquePtr<const Policy> aPolicy, int aChildPid,
                 int& aClientFd);
   void ThreadMain(void) override;
   void AuditDenial(int aOp, int aFlags, int aPerms, const char* aPath);
+  // Remap relative paths to absolute paths.
+  size_t ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen);
 
   // Holding a UniquePtr should disallow copying, but to make that explicit:
   SandboxBroker(const SandboxBroker&) = delete;
   void operator=(const SandboxBroker&) = delete;
 };
 
 } // namespace mozilla