Fix memory smashing on the sandbox PolicyRule

PolicyRule copy ctor was not taking into account that the source policy rule
could be using some 'constants' memory at the bottom, so adding further
opcodes to the new policy rule would overwrite the copied ones.

In other words, this pattern

PolicyRule pr_orig(ASK_BROKER);
pr_orig.AddStringMatch(...);
PolicyRule pr_copy(pr_orig);
pr_copy.AddStringMatch(...);

Was broken. This was not impacting the chrome sbox code because we don't
mutate the new rule after copy construction.

Acknoledgments to Ashutosh Mehra from Adobe Corp for pointing the bug
and providing a test case.

BUG=160890
TEST=new unittest added
Review URL: https://codereview.chromium.org/11275301

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167571 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 52 insertions, 10 deletions

diff --git a/sandbox/win/src/policy_engine_opcodes.h b/sandbox/win/src/policy_engine_opcodes.h
index f74ce31..306e677 100644
--- a/sandbox/win/src/policy_engine_opcodes.h
+++ b/sandbox/win/src/policy_engine_opcodes.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SANDBOX_SRC_POLICY_ENGINE_OPCODES_H__
-#define SANDBOX_SRC_POLICY_ENGINE_OPCODES_H__
+#ifndef SANDBOX_WIN_SRC_POLICY_ENGINE_OPCODES_H_
+#define SANDBOX_WIN_SRC_POLICY_ENGINE_OPCODES_H_
 
 #include "sandbox/win/src/policy_engine_params.h"
 #include "base/basictypes.h"
@@ -287,6 +287,11 @@ class OpcodeFactory {
     memory_bottom_ = &memory_top_[memory_size];
   }
 
+  // Returns the available memory to make opcodes.
+  size_t memory_size() const {
+    return memory_bottom_ - memory_top_;
+  }
+
   // Creates an OpAlwaysFalse opcode.
   PolicyOpcode* MakeOpAlwaysFalse(uint32 options);
 
@@ -358,11 +363,6 @@ class OpcodeFactory {
   // returns the displacement with respect to start.
   ptrdiff_t AllocRelative(void* start, const wchar_t* str, size_t lenght);
 
-  // Returns the available memory to make opcodes.
-  size_t memory_size() const {
-    return memory_bottom_ - memory_top_;
-  }
-
   // Points to the lowest currently available address of the memory
   // used to make the opcodes. This pointer increments as opcodes are made.
   char* memory_top_;
@@ -377,4 +377,4 @@ class OpcodeFactory {
 
 }  // namespace sandbox
 
-#endif  // SANDBOX_SRC_POLICY_ENGINE_OPCODES_H__
+#endif  // SANDBOX_WIN_SRC_POLICY_ENGINE_OPCODES_H_
diff --git a/sandbox/win/src/policy_low_level.cc b/sandbox/win/src/policy_low_level.cc
index 8431bc0..686caa1 100644
--- a/sandbox/win/src/policy_low_level.cc
+++ b/sandbox/win/src/policy_low_level.cc
@@ -136,9 +136,9 @@ PolicyRule::PolicyRule(const PolicyRule& other) {
   memcpy(buffer_, other.buffer_, buffer_size);
 
   char* opcode_buffer = reinterpret_cast<char*>(&buffer_->opcodes[0]);
-  char* buffer_end = &opcode_buffer[kRuleBufferSize + sizeof(PolicyOpcode)];
   char* next_opcode = &opcode_buffer[GetOpcodeCount() * sizeof(PolicyOpcode)];
-  opcode_factory_ = new OpcodeFactory(next_opcode, buffer_end - next_opcode);
+  opcode_factory_ =
+      new OpcodeFactory(next_opcode, other.opcode_factory_->memory_size());
 }
 
 // This function get called from a simple state machine implemented in
diff --git a/sandbox/win/src/policy_low_level_unittest.cc b/sandbox/win/src/policy_low_level_unittest.cc
index 4a2cf4b..2b5d0f7 100644
--- a/sandbox/win/src/policy_low_level_unittest.cc
+++ b/sandbox/win/src/policy_low_level_unittest.cc
@@ -572,4 +572,46 @@ TEST(PolicyEngineTest, ThreeRulesTest) {
   delete [] reinterpret_cast<char*>(policy);
 }
 
+TEST(PolicyEngineTest, PolicyRuleCopyConstructorTwoStrings) {
+  SetupNtdllImports();
+  // Both pr_orig and pr_copy should allow hello.* but not *.txt files.
+  PolicyRule pr_orig(ASK_BROKER);
+  EXPECT_TRUE(pr_orig.AddStringMatch(IF, 0, L"hello.*", CASE_SENSITIVE));
+
+  PolicyRule pr_copy(pr_orig);
+  EXPECT_TRUE(pr_orig.AddStringMatch(IF_NOT, 0, L"*.txt", CASE_SENSITIVE));
+  EXPECT_TRUE(pr_copy.AddStringMatch(IF_NOT, 0, L"*.txt", CASE_SENSITIVE));
+
+  PolicyGlobal* policy = MakePolicyMemory();
+  LowLevelPolicy policyGen(policy);
+  EXPECT_TRUE(policyGen.AddRule(1, &pr_orig));
+  EXPECT_TRUE(policyGen.AddRule(2, &pr_copy));
+  EXPECT_TRUE(policyGen.Done());
+
+  wchar_t* name = NULL;
+  POLPARAMS_BEGIN(eval_params)
+    POLPARAM(name)
+  POLPARAMS_END;
+
+  PolicyResult result;
+  PolicyProcessor pol_ev_orig(policy->entry[1]);
+  name = L"domo.txt";
+  result = pol_ev_orig.Evaluate(kShortEval, eval_params, _countof(eval_params));
+  EXPECT_EQ(NO_POLICY_MATCH, result);
+
+  name = L"hello.bmp";
+  result = pol_ev_orig.Evaluate(kShortEval, eval_params, _countof(eval_params));
+  EXPECT_EQ(POLICY_MATCH, result);
+  EXPECT_EQ(ASK_BROKER, pol_ev_orig.GetAction());
+
+  PolicyProcessor pol_ev_copy(policy->entry[2]);
+  name = L"domo.txt";
+  result = pol_ev_copy.Evaluate(kShortEval, eval_params, _countof(eval_params));
+  EXPECT_EQ(NO_POLICY_MATCH, result);
+
+  name = L"hello.bmp";
+  result = pol_ev_copy.Evaluate(kShortEval, eval_params, _countof(eval_params));
+  EXPECT_EQ(POLICY_MATCH, result);
+  EXPECT_EQ(ASK_BROKER, pol_ev_copy.GetAction());
+}
 }  // namespace sandbox