Replace NT prefix in sandbox rules match string to handle correct wildcard escaping

This patch adds a function to modify file system sandbox rules to replace the
\??\ NT prefix with the correct escaped form \/?/?\ for the wildcard matching
rules in the broker. This is done generally as it's a common mistake in the
sandbox code and so provides some defence in depth.

BUG=334882

Review URL: https://codereview.chromium.org/432543005

Cr-Commit-Position: refs/heads/master@{#290131}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290131 0039d316-1c4b-4281-b951-d872f2087c98

sandbox/win/src/file_policy_test.cc

@@ -9,6 +9,7 @@
 #include <winioctl.h>
 
 #include "base/win/scoped_handle.h"
+#include "sandbox/win/src/filesystem_policy.h"
 #include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/sandbox_factory.h"
@@ -596,4 +597,28 @@ TEST(FilePolicyTest, DISABLED_TestReparsePoint) {
   EXPECT_TRUE(::RemoveDirectory(subfolder.c_str()));
 }
 
+TEST(FilePolicyTest, CheckExistingNTPrefixEscape) {
+  base::string16 name = L"\\??\\NAME";
+
+  base::string16 result = FixNTPrefixForMatch(name);
+
+  EXPECT_STREQ(result.c_str(), L"\\/?/?\\NAME");
+}
+
+TEST(FilePolicyTest, CheckEscapedNTPrefixNoEscape) {
+  base::string16 name = L"\\/?/?\\NAME";
+
+  base::string16 result = FixNTPrefixForMatch(name);
+
+  EXPECT_STREQ(result.c_str(), name.c_str());
+}
+
+TEST(FilePolicyTest, CheckMissingNTPrefixEscape) {
+  base::string16 name = L"C:\\NAME";
+
+  base::string16 result = FixNTPrefixForMatch(name);
+
+  EXPECT_STREQ(result.c_str(), L"\\/?/?\\C:\\NAME");
+}
+
 }  // namespace sandbox

sandbox/win/src/filesystem_policy.cc

@@ -77,12 +77,9 @@ bool FileSystemPolicy::GenerateRules(const wchar_t* name,
       NOTREACHED();
       return false;
     }
-    if (0 != mod_name.compare(0, kNTPrefixLen, kNTPrefix)) {
-      // TODO(nsylvain): Find a better way to do name resolution. Right now we
-      // take the name and we expand it.
-      mod_name.insert(0, L"\\/?/?\\");
-      name = mod_name.c_str();
-    }
+
+    mod_name = FixNTPrefixForMatch(mod_name);
+    name = mod_name.c_str();
   }
 
   EvalResult result = ASK_BROKER;
@@ -383,4 +380,26 @@ bool PreProcessName(const base::string16& path, base::string16* new_path) {
   return !reparsed;
 }
 
+base::string16 FixNTPrefixForMatch(const base::string16& name) {
+  base::string16 mod_name = name;
+
+  // NT prefix escaped for rule matcher
+  const wchar_t kNTPrefixEscaped[] = L"\\/?/?\\";
+  const int kNTPrefixEscapedLen = arraysize(kNTPrefixEscaped) - 1;
+
+  if (0 != mod_name.compare(0, kNTPrefixLen, kNTPrefix)) {
+    if (0 != mod_name.compare(0, kNTPrefixEscapedLen, kNTPrefixEscaped)) {
+      // TODO(nsylvain): Find a better way to do name resolution. Right now we
+      // take the name and we expand it.
+      mod_name.insert(0, kNTPrefixEscaped);
+    }
+  } else {
+    // Start of name matches NT prefix, replace with escaped format
+    // Fixes bug: 334882
+    mod_name.replace(0, kNTPrefixLen, kNTPrefixEscaped);
+  }
+
+  return mod_name;
+}
+
 }  // namespace sandbox

sandbox/win/src/filesystem_policy.h

@@ -103,6 +103,11 @@ class FileSystemPolicy {
 // the path cannot be trusted.
 bool PreProcessName(const base::string16& path, base::string16* new_path);
 
+// Corrects global paths to have a correctly escaped NT prefix at the
+// beginning. If the name has no NT prefix (either normal or escaped)
+// add the escaped form to the string
+base::string16 FixNTPrefixForMatch(const base::string16& name);
+
 }  // namespace sandbox
 
 #endif  // SANDBOX_SRC_FILESYSTEM_POLICY_H__