Add regex escaping code to Mac sandbox implementation and re-enable the utility process on OS X.

chrome/common/sandbox_mac.mm

Index: chrome/common/sandbox_mac.mm
diff --git a/chrome/common/sandbox_mac.mm b/chrome/common/sandbox_mac.mm
index 6eac34c3ae5c7460f9b7ca95d6e17d510dfee334..686420684f4d52de1541a4d3354cbbb95ff13729 100644
--- a/chrome/common/sandbox_mac.mm
+++ b/chrome/common/sandbox_mac.mm
@@ -13,17 +13,163 @@ extern "C" {
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
-#include "base/json/string_escape.h"
+#include "base/file_util.h"
 #include "base/mac_util.h"
 #include "base/scoped_cftyperef.h"
 #include "base/scoped_nsautorelease_pool.h"
 #include "base/string16.h"
 #include "base/sys_info.h"
 #include "base/sys_string_conversions.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
+#include "unicode/uchar.h"
+
+namespace {
+
+// Try to escape |c| as a "SingleEscapeCharacter" (\n, etc).  If successful,
+// returns true and appends the escape sequence to |dst|.
+bool EscapeSingleChar(char c, std::string* dst) {

[Mark Mentovai, 2009/12/03 14:11:25]

I think you should only have one dst->append in this function, right before
return true.

Outside of the switch, you could have |const char* append = NULL;|, and then for
each case that wants to append something, |append = "\\b";| (or whatever).

+  switch (c) {
+    case '\b':
+      dst->append("\\b");
+      break;
+    case '\f':
+      dst->append("\\f");
+      break;
+    case '\n':
+      dst->append("\\n");
+      break;
+    case '\r':
+      dst->append("\\r");
+      break;
+    case '\t':
+      dst->append("\\t");
+      break;
+    case '\\':
+      dst->append("\\\\");
+      break;
+    case '"':
+      dst->append("\\\"");
+      break;
+    default:
+      return false;
+  }
+  return true;
+}
+
+}  // namespace
 
 namespace sandbox {
 
+// Escape |str_utf8| for use in a plain string variable in a sandbox
+// configuraton file.  On return |dst| is set to the utf-8 encoded quoted
+// output.
+// Returns: true on success, false otherwise.
+bool QuotePlainString(const std::string& str_utf8, std::string* dst) {
+  const char* src = str_utf8.c_str();
+  int32_t length = str_utf8.length();
+  int32_t position = 0;
+  while (position < length) {

[Mark Mentovai, 2009/12/03 14:11:25]

Before entering the loop, clear dst.

+    UChar32 c;
+    U8_NEXT(src, position, length, c);  // Macro increments |position|.
+    DCHECK_GE(c, 0);
+    if (c < 0)
+      return false;
+
+    if (c < 128) {  // EscapeSingleChar only handles ASCII.
+      char as_char = static_cast<char>(c);
+      if (EscapeSingleChar(as_char, dst)) {
+        continue;
+      }
+    }
+
+    if (c < 32 || c > 126) {
+      // Any characters that aren't printable ASCII get the \u treatment.
+      unsigned int as_uint = static_cast<unsigned int>(c);
+      StringAppendF(dst, "\\u%04X", as_uint);
+      continue;
+    }
+
+    // If we got here we know that the character in question is strictly
+    // in the ASCII range so there's no need to do any kind of encoding
+    // conversion.
+    dst->push_back(static_cast<char>(c));
+  }
+  return true;
+}
+
+// Escape |str_utf8| for use in a regex literal in a sandbox
+// configuraton file.  On return |dst| is set to the utf-8 encoded quoted
+// output.
+//
+// The implementation of this function is based on empirical testing of the
+// OS X sandbox on 10.5.8 & 10.6.2 which is undocumented and subject to change.
+//
+// Note: If str_utf8 contains any characters < 32 || >125 then the function
+// fails and false is returned.
+//
+// Returns: true on success, false otherwise.
+bool QuoteStringForRegex(const std::string& str_utf8, std::string* dst) {
+  // List of chars with special meaning to regex.
+  // This list is derived from http://perldoc.perl.org/perlre.html .
+  const char regex_special_chars[] = {
+    '\\',
+
+    // Metacharacters
+    '^',
+    '.',
+    '$',
+    '|',
+    '(',
+    ')',
+    '[',
+    ']',
+
+    // Quantifiers
+    '*',
+    '+',
+    '?',
+    '{',
+    '}',
+  };
+
+  // Anchor regex at start of path.
+  dst->push_back('^');

[Mark Mentovai, 2009/12/03 14:11:25]

Don't push_back.  Assign.  You want to clear out dst before you add anything to
it.

+
+  const char* src = str_utf8.c_str();
+  int32_t length = str_utf8.length();
+  int32_t position = 0;
+  while (position < length) {
+    UChar32 c;
+    U8_NEXT(src, position, length, c);  // Macro increments |position|.
+    DCHECK_GE(c, 0);
+    if (c < 0)
+      return false;
+
+    // The Mac sandbox regex parser only handles printable ASCII characters.
+    // 33 >= c <= 126
+    if (c < 32 || c > 125) {
+      return false;
+    }
+
+    for (size_t i = 0; i < arraysize(regex_special_chars); ++i) {
+      if (c == regex_special_chars[i]) {
+        dst->push_back('\\');
+        break;
+      }
+    }
+
+    dst->push_back(static_cast<char>(c));
+  }
+
+  // Make sure last element of path is interpreted as a directory, leaving this

[Mark Mentovai, 2009/12/03 14:11:25]

as a directory.  Leaving this

(period and a new sentence.)

+  // off would allow access to files if they start with the same name as the
+  // directory.
+  dst->append("(/|$)");
+
+  return true;
+}
+
 // Warm up System APIs that empirically need to be accessed before the Sandbox
 // is turned on.
 // This method is layed out in blocks, each one containing a separate function
@@ -91,12 +237,7 @@ bool EnableSandbox(SandboxProcessType sandbox_type,
   if (sandbox_type != SANDBOX_TYPE_UTILITY) {
     DCHECK(allowed_dir.empty())
         << "Only SANDBOX_TYPE_UTILITY allows a custom directory parameter.";
-  } else {
-    DCHECK(!allowed_dir.empty())
-        << "SANDBOX_TYPE_UTILITY "
-        << "needs a custom directory parameter, but an empty one was provided.";
   }
-
   // We use a custom sandbox definition file to lock things down as
   // tightly as possible.
   // TODO(jeremy): Look at using include syntax to unify common parts of sandbox
@@ -126,7 +267,8 @@ bool EnableSandbox(SandboxProcessType sandbox_type,
       error:nil];
 
   if (!sandbox_data) {
-    LOG(ERROR) << "Failed to find the sandbox profile on disk";
+    PLOG(ERROR) << "Failed to find the sandbox profile on disk "
+        << base::SysNSStringToUTF8(sandbox_profile_path);

[Mark Mentovai, 2009/12/03 14:11:25]

Align <<s.

     return false;
   }
 
@@ -140,10 +282,31 @@ bool EnableSandbox(SandboxProcessType sandbox_type,
   }
 
   if (!allowed_dir.empty()) {
-    NSString* allowed_dir_ns = base::SysUTF8ToNSString(allowed_dir.value());
+    // The sandbox only understands "real" paths.  This resolving step is
+    // needed so the caller doesn't need to worry about things like /var
+    // being a link to /private/var (like in the paths CreateNewTempDirectory()
+    // returns).
+    FilePath allowed_dir_absolute(allowed_dir);
+    if (!file_util::AbsolutePath(&allowed_dir_absolute)) {
+      PLOG(ERROR) << "Failed to resolve absolute path error: ";

[Mark Mentovai, 2009/12/03 14:11:25]

PLOG will take care of appending the error and any other stuff it needs to make
sense out of the string.  This just needs to be:

      PLOG(ERROR) << "Failed to resolve absolute path";

+      return false;
+    }
+
+    std::string allowed_dir_escaped;
+    if (!QuoteStringForRegex(allowed_dir_absolute.value(),
+            &allowed_dir_escaped)) {

[Mark Mentovai, 2009/12/03 14:11:25]

&allowed_dir_excaped should line up with allowed_dir_absolute.

+      LOG(ERROR) << "Regex string quoting failed " << allowed_dir.value();
+      return false;
+    }
+    NSString* allowed_dir_escaped_ns = base::SysUTF8ToNSString(
+        allowed_dir_escaped.c_str());
+    sandbox_data = [sandbox_data
+        stringByReplacingOccurrencesOfString:@";ENABLE_DIRECTORY_ACCESS"
+                                  withString:@""];
     sandbox_data = [sandbox_data
         stringByReplacingOccurrencesOfString:@"DIR_TO_ALLOW_ACCESS"
-                                  withString:allowed_dir_ns];
+                                  withString:allowed_dir_escaped_ns];
+
   }
 
   int32 major_version, minor_version, bugfix_version;
@@ -161,9 +324,12 @@ bool EnableSandbox(SandboxProcessType sandbox_type,
     // for this "subdir" is only supported on 10.6.
     // If we ever need this on pre-10.6 OSs then we'll have to rethink the
     // surrounding sandbox syntax.
-    string16 home_dir = base::SysNSStringToUTF16(NSHomeDirectory());
+    std::string home_dir = base::SysNSStringToUTF8(NSHomeDirectory());
     std::string home_dir_escaped;
-    base::JsonDoubleQuote(home_dir, false, &home_dir_escaped);
+    if (!QuotePlainString(home_dir, &home_dir_escaped)) {
+      LOG(ERROR) << "Sandbox string quoting failed";
+      return false;
+    }
     NSString* home_dir_escaped_ns = base::SysUTF8ToNSString(home_dir_escaped);
     sandbox_data = [sandbox_data
         stringByReplacingOccurrencesOfString:@"USER_HOMEDIR"
@@ -173,9 +339,10 @@ bool EnableSandbox(SandboxProcessType sandbox_type,
   char* error_buff = NULL;
   int error = sandbox_init([sandbox_data UTF8String], 0, &error_buff);
   bool success = (error == 0 && error_buff == NULL);
-  if (error == -1) {
-    LOG(ERROR) << "Failed to Initialize Sandbox: " << error_buff;
-  }
+  LOG_IF(ERROR, !success) << "Failed to Initialize Sandbox: "

[Mark Mentovai, 2009/12/03 14:11:25]

initialize and sandbox should be lowercase.

+                          << error
+                          << " "
+                          << error_buff;
   sandbox_free_error(error_buff);
   return success;
 }