Avoid std::string copying in GetFileNameInWhitelist.

sandbox/linux/services/broker_process.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sandbox/linux/services/broker_process.h"
 
 #include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "build/build_config.h"
+#include "sandbox/linux/services/broker_process_utils.h"
 #include "sandbox/linux/services/linux_syscalls.h"
 
 #if defined(OS_ANDROID) && !defined(MSG_CMSG_CLOEXEC)
 #define MSG_CMSG_CLOEXEC 0x40000000
 #endif
 
 namespace {
 
 static const size_t kMaxMessageLength = 4096;
 
@@ skipping 12 matching lines @@
 // Since we have to account for buggy userland (see crbug.com/237283), we will
 // open(2) the file with O_CLOEXEC in the broker process if necessary, in
 // addition to calling recvmsg(2) with MSG_CMSG_CLOEXEC.
 static const int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
 
 // Check whether |requested_filename| is in |allowed_file_names|.
 // See GetFileNameIfAllowedToOpen() for an explanation of |file_to_open|.
 // async signal safe if |file_to_open| is NULL.
 // TODO(jln): assert signal safety.
 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
-                            const std::string& requested_filename,
+                            const char* requested_filename,
                             const char** file_to_open) {
   if (file_to_open && *file_to_open) {
     // Make sure that callers never pass a non-empty string. In case callers
     // wrongly forget to check the return value and look at the string
     // instead, this could catch bugs.
     RAW_LOG(FATAL, "*file_to_open should be NULL");
     return false;
   }
-  std::vector<std::string>::const_iterator it;
-  it = std::find(allowed_file_names.begin(), allowed_file_names.end(),
-                 requested_filename);
-  if (it < allowed_file_names.end()) {  // requested_filename was found?
+  const char* found_filename =
+      sandbox::BrokerProcessUtils::Find(requested_filename, allowed_file_names);
+
+  if (found_filename) {  // |requested_filename| was found?
     if (file_to_open)
-      *file_to_open = it->c_str();
+      *file_to_open = found_filename;
     return true;
   }
   return false;
 }
 
 // We maintain a list of flags that have been reviewed for "sanity" and that
 // we're ok to allow in the broker.
 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM.
 bool IsAllowedOpenFlags(int flags) {
   // First, check the access mode
@@ skipping 305 matching lines @@
 }
 
 // Perform access(2) on |requested_filename| with mode |mode| if allowed by our
 // policy. Write the syscall return value (-errno) to |write_pickle|.
 void BrokerProcess::AccessFileForIPC(const std::string& requested_filename,
                                      int mode, Pickle* write_pickle) const {
   DCHECK(write_pickle);
   const char* file_to_access = NULL;
   const bool safe_to_access_file = GetFileNameIfAllowedToAccess(
       requested_filename.c_str(), mode, &file_to_access);
+
   if (safe_to_access_file) {
     CHECK(file_to_access);
     int access_ret = access(file_to_access, mode);
     int access_errno = errno;
     if (!access_ret)
       write_pickle->WriteInt(0);
     else
       write_pickle->WriteInt(-access_errno);
   } else {
     write_pickle->WriteInt(-EPERM);
@@ skipping 34 matching lines @@
 // mode |requested_mode|.
 // Note: access() being a system call to check permissions, this can get a bit
 // confusing. We're checking if calling access() should even be allowed with
 // the same policy we would use for open().
 // If |file_to_access| is not NULL, we will return the matching pointer from
 // the whitelist. For paranoia a caller should then use |file_to_access|. See
 // GetFileNameIfAllowedToOpen() fore more explanation.
 // return true if calling access() on this file should be allowed, false
 // otherwise.
 // Async signal safe if and only if |file_to_access| is NULL.
-bool BrokerProcess::GetFileNameIfAllowedToAccess(const char* requested_filename,
-    int requested_mode, const char** file_to_access) const {

[jln (very slow on Chromium), 2013/07/08 22:36:51]

Why the re-indent?

[Jorge Lucangeli Obes, 2013/07/09 03:52:12]

Done.

+bool BrokerProcess::GetFileNameIfAllowedToAccess(
+    const char* requested_filename, int requested_mode,
+    const char** file_to_access) const {
   // First, check if |requested_mode| is existence, ability to read or ability
   // to write. We do not support X_OK.
   if (requested_mode != F_OK &&
       requested_mode & ~(R_OK | W_OK)) {
     return false;
   }
   switch (requested_mode) {
     case F_OK:
       // We allow to check for file existence if we can either read or write.
       return GetFileNameInWhitelist(allowed_r_files_, requested_filename,
@@ skipping 20 matching lines @@
 }
 
 // Check if |requested_filename| can be opened with flags |requested_flags|.
 // If |file_to_open| is not NULL, we will return the matching pointer from the
 // whitelist. For paranoia, a caller should then use |file_to_open| rather
 // than |requested_filename|, so that it never attempts to open an
 // attacker-controlled file name, even if an attacker managed to fool the
 // string comparison mechanism.
 // Return true if opening should be allowed, false otherwise.
 // Async signal safe if and only if |file_to_open| is NULL.
-bool BrokerProcess::GetFileNameIfAllowedToOpen(const char* requested_filename,
-    int requested_flags, const char** file_to_open) const {
+bool BrokerProcess::GetFileNameIfAllowedToOpen(
+    const char* requested_filename, int requested_flags,
+    const char** file_to_open) const {

[jln (very slow on Chromium), 2013/07/08 22:36:51]

Why the re-indent?

[Jorge Lucangeli Obes, 2013/07/09 03:52:12]

Done.

   if (!IsAllowedOpenFlags(requested_flags)) {
     return false;
   }
   switch (requested_flags & O_ACCMODE) {
     case O_RDONLY:
       return GetFileNameInWhitelist(allowed_r_files_, requested_filename,
                                     file_to_open);
     case O_WRONLY:
       return GetFileNameInWhitelist(allowed_w_files_, requested_filename,
                                     file_to_open);
     case O_RDWR:
     {
       bool allowed_for_read_and_write =
           GetFileNameInWhitelist(allowed_r_files_, requested_filename, NULL) &&
           GetFileNameInWhitelist(allowed_w_files_, requested_filename,
                                  file_to_open);
       return allowed_for_read_and_write;
     }
     default:
       return false;
   }
 }
 
 }  // namespace sandbox.