Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(336)

Side by Side Diff: sandbox/linux/services/broker_process.cc

Issue 18337010: Avoid std::string copying in GetFileNameInWhitelist. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Small whitespace consistency fix. Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sandbox/linux/services/broker_process.h" 5 #include "sandbox/linux/services/broker_process.h"
6 6
7 #include <fcntl.h> 7 #include <fcntl.h>
8 #include <sys/socket.h> 8 #include <sys/socket.h>
9 #include <sys/stat.h> 9 #include <sys/stat.h>
10 #include <sys/syscall.h> 10 #include <sys/syscall.h>
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
46 // Since we have to account for buggy userland (see crbug.com/237283), we will 46 // Since we have to account for buggy userland (see crbug.com/237283), we will
47 // open(2) the file with O_CLOEXEC in the broker process if necessary, in 47 // open(2) the file with O_CLOEXEC in the broker process if necessary, in
48 // addition to calling recvmsg(2) with MSG_CMSG_CLOEXEC. 48 // addition to calling recvmsg(2) with MSG_CMSG_CLOEXEC.
49 static const int kCurrentProcessOpenFlagsMask = O_CLOEXEC; 49 static const int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
50 50
51 // Check whether |requested_filename| is in |allowed_file_names|. 51 // Check whether |requested_filename| is in |allowed_file_names|.
52 // See GetFileNameIfAllowedToOpen() for an explanation of |file_to_open|. 52 // See GetFileNameIfAllowedToOpen() for an explanation of |file_to_open|.
53 // async signal safe if |file_to_open| is NULL. 53 // async signal safe if |file_to_open| is NULL.
54 // TODO(jln): assert signal safety. 54 // TODO(jln): assert signal safety.
55 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names, 55 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
56 const std::string& requested_filename, 56 const char* requested_filename,
57 const char** file_to_open) { 57 const char** file_to_open) {
58 if (file_to_open && *file_to_open) { 58 if (file_to_open && *file_to_open) {
59 // Make sure that callers never pass a non-empty string. In case callers 59 // Make sure that callers never pass a non-empty string. In case callers
60 // wrongly forget to check the return value and look at the string 60 // wrongly forget to check the return value and look at the string
61 // instead, this could catch bugs. 61 // instead, this could catch bugs.
62 RAW_LOG(FATAL, "*file_to_open should be NULL"); 62 RAW_LOG(FATAL, "*file_to_open should be NULL");
63 return false; 63 return false;
64 } 64 }
65
66 // Look for |requested_filename| in |allowed_file_names|.
67 // We don't use ::find() because it takes a std::string and
68 // the conversion allocates memory.
65 std::vector<std::string>::const_iterator it; 69 std::vector<std::string>::const_iterator it;
66 it = std::find(allowed_file_names.begin(), allowed_file_names.end(), 70 for (it = allowed_file_names.begin(); it != allowed_file_names.end(); it++) {
67 requested_filename); 71 if (strcmp(requested_filename, it->c_str()) == 0) {
68 if (it < allowed_file_names.end()) { // requested_filename was found? 72 if (file_to_open)
69 if (file_to_open) 73 *file_to_open = it->c_str();
70 *file_to_open = it->c_str(); 74 return true;
71 return true; 75 }
72 } 76 }
73 return false; 77 return false;
74 } 78 }
75 79
76 // We maintain a list of flags that have been reviewed for "sanity" and that 80 // We maintain a list of flags that have been reviewed for "sanity" and that
77 // we're ok to allow in the broker. 81 // we're ok to allow in the broker.
78 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM. 82 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM.
79 bool IsAllowedOpenFlags(int flags) { 83 bool IsAllowedOpenFlags(int flags) {
80 // First, check the access mode 84 // First, check the access mode
81 const int access_mode = flags & O_ACCMODE; 85 const int access_mode = flags & O_ACCMODE;
(...skipping 304 matching lines...) Expand 10 before | Expand all | Expand 10 after
386 } 390 }
387 391
388 // Perform access(2) on |requested_filename| with mode |mode| if allowed by our 392 // Perform access(2) on |requested_filename| with mode |mode| if allowed by our
389 // policy. Write the syscall return value (-errno) to |write_pickle|. 393 // policy. Write the syscall return value (-errno) to |write_pickle|.
390 void BrokerProcess::AccessFileForIPC(const std::string& requested_filename, 394 void BrokerProcess::AccessFileForIPC(const std::string& requested_filename,
391 int mode, Pickle* write_pickle) const { 395 int mode, Pickle* write_pickle) const {
392 DCHECK(write_pickle); 396 DCHECK(write_pickle);
393 const char* file_to_access = NULL; 397 const char* file_to_access = NULL;
394 const bool safe_to_access_file = GetFileNameIfAllowedToAccess( 398 const bool safe_to_access_file = GetFileNameIfAllowedToAccess(
395 requested_filename.c_str(), mode, &file_to_access); 399 requested_filename.c_str(), mode, &file_to_access);
400
396 if (safe_to_access_file) { 401 if (safe_to_access_file) {
397 CHECK(file_to_access); 402 CHECK(file_to_access);
398 int access_ret = access(file_to_access, mode); 403 int access_ret = access(file_to_access, mode);
399 int access_errno = errno; 404 int access_errno = errno;
400 if (!access_ret) 405 if (!access_ret)
401 write_pickle->WriteInt(0); 406 write_pickle->WriteInt(0);
402 else 407 else
403 write_pickle->WriteInt(-access_errno); 408 write_pickle->WriteInt(-access_errno);
404 } else { 409 } else {
405 write_pickle->WriteInt(-EPERM); 410 write_pickle->WriteInt(-EPERM);
(...skipping 102 matching lines...) Expand 10 before | Expand all | Expand 10 after
508 GetFileNameInWhitelist(allowed_w_files_, requested_filename, 513 GetFileNameInWhitelist(allowed_w_files_, requested_filename,
509 file_to_open); 514 file_to_open);
510 return allowed_for_read_and_write; 515 return allowed_for_read_and_write;
511 } 516 }
512 default: 517 default:
513 return false; 518 return false;
514 } 519 }
515 } 520 }
516 521
517 } // namespace sandbox. 522 } // namespace sandbox.
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698