summaryrefslogtreecommitdiffstats
path: root/sandbox
diff options
context:
space:
mode:
authorcpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-20 01:27:20 +0000
committercpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-08-20 01:27:20 +0000
commitaf1e6f59e12fabc8ae8ab1fbb0f0423e42dfadf2 (patch)
treed8e2d84f8b6baeb4bcf7e44cc9f2dcf73bcfbcaf /sandbox
parent0c5970256ba27bbc97d03586646c5931374188ca (diff)
downloadchromium_src-af1e6f59e12fabc8ae8ab1fbb0f0423e42dfadf2.zip
chromium_src-af1e6f59e12fabc8ae8ab1fbb0f0423e42dfadf2.tar.gz
chromium_src-af1e6f59e12fabc8ae8ab1fbb0f0423e42dfadf2.tar.bz2
Revert 56796 - Sbox IPC fix
Tests failing on vista BUG=52682 TEST=included Review URL: http://codereview.chromium.org/3142022 TBR=cpu@chromium.org Review URL: http://codereview.chromium.org/3122031 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56798 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sandbox')
-rw-r--r--sandbox/src/crosscall_params.h51
-rw-r--r--sandbox/src/crosscall_server.cc43
-rw-r--r--sandbox/src/ipc_unittest.cc17
3 files changed, 39 insertions, 72 deletions
diff --git a/sandbox/src/crosscall_params.h b/sandbox/src/crosscall_params.h
index 4c1e4fe..939925c 100644
--- a/sandbox/src/crosscall_params.h
+++ b/sandbox/src/crosscall_params.h
@@ -169,33 +169,30 @@ class CrossCallParams {
// blob to be complex.
//
// As is, this class assumes that the layout of the blob is as follows. Assume
-// that NUMBER_PARAMS = 2 and a 32-bit build:
+// that NUMBER_PARAMS = 2:
//
-// [ tag 4 bytes]
-// [ IsOnOut 4 bytes]
-// [ call return 52 bytes]
-// [ params count 4 bytes]
-// [ parameter 0 type 4 bytes]
-// [ parameter 0 offset 4 bytes] ---delta to ---\
-// [ parameter 0 size 4 bytes] |
-// [ parameter 1 type 4 bytes] |
-// [ parameter 1 offset 4 bytes] ---------------|--\
-// [ parameter 1 size 4 bytes] | |
-// [ parameter 2 type 4 bytes] | |
-// [ parameter 2 offset 4 bytes] ----------------------\
-// [ parameter 2 size 4 bytes] | | |
-// |---------------------------| | | |
-// | value 0 (x bytes) | <--------------/ | |
-// | value 1 (y bytes) | <-----------------/ |
-// | | |
-// | end of buffer | <---------------------/
-// |---------------------------|
+// [ tag 4 bytes]
+// [ IsOnOut 4 bytes]
+// [ call return 52 bytes]
+// [ params count 4 bytes]
+// [ parameter 0 type 4 bytes]
+// [ parameter 0 /\ 4 bytes] ---delta to ---\
+// [ parameter 0 size 4 bytes] |
+// [ parameter 1 type 4 bytes] |
+// [ parameter 1 /\ 4 bytes] |
+// [ parameter 1 size 4 bytes] |
+// [ parameter 2 type 4 bytes] |
+// [ parameter 2 /\ 4 bytes] ----------------------\
+// [ parameter 2 size 4 bytes] | |
+// |-------------------------| | |
+// | | <--------------/ |
+// | | |
+// | | <---------------------/
+// |-------------------------|
//
// Note that the actual number of params is NUMBER_PARAMS + 1
// so that the size of each actual param can be computed from the difference
-// between one parameter and the next down. The offset of the last param
-// points to the end of the buffer and the type and size are undefined.
-//
+// between one parameter and the next down
template <size_t NUMBER_PARAMS, size_t BLOCK_SIZE>
class ActualCallParams : public CrossCallParams {
public:
@@ -212,14 +209,6 @@ class ActualCallParams : public CrossCallParams {
param_info_[0].offset_ = parameters_ - reinterpret_cast<char*>(this);
}
- // Testing-only method. Allows setting the apparent size to a wrong value.
- // returns the previous size.
- size_t OverrideSize(size_t new_size) {
- size_t previous_size = param_info_[NUMBER_PARAMS].offset_;
- param_info_[NUMBER_PARAMS].offset_ = new_size;
- return previous_size;
- }
-
// Copies each paramter into the internal buffer. For each you must supply:
// index: 0 for the first param, 1 for the next an so on
bool CopyParamIn(size_t index, const void* parameter_address, size_t size,
diff --git a/sandbox/src/crosscall_server.cc b/sandbox/src/crosscall_server.cc
index 5063ce6..da43c69 100644
--- a/sandbox/src/crosscall_server.cc
+++ b/sandbox/src/crosscall_server.cc
@@ -85,8 +85,6 @@ void CrossCallParamsEx::operator delete(void* raw_memory) throw() {
CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base,
size_t buffer_size,
size_t* output_size) {
- // IMPORTANT: Everything inside buffer_base and derived from it such
- // as param_count and declared_size is untrusted.
if (NULL == buffer_base) {
return NULL;
}
@@ -96,12 +94,10 @@ CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base,
if (buffer_size > kMaxBufferSize) {
return NULL;
}
-
char* backing_mem = NULL;
size_t param_count = 0;
- size_t declared_size;
- size_t min_declared_size;
CrossCallParamsEx* copied_params = NULL;
+ size_t actual_size;
// Touching the untrusted buffer is done under a SEH try block. This
// will catch memory access violations so we don't crash.
@@ -111,28 +107,30 @@ CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base,
// Check against the minimum size given the number of stated params
// if too small we bail out.
param_count = call_params->GetParamsCount();
-
- min_declared_size =
- sizeof(CrossCallParamsEx) + (param_count * sizeof(ParamInfo));
-
- if (min_declared_size < sizeof(CrossCallParams) ||
- (buffer_size < min_declared_size)) {
- // Integer overflow or computed size bigger than untrusted buffer.
+ if ((buffer_size - sizeof(CrossCallParams)) <
+ (sizeof(ptrdiff_t) * (param_count + 1))) {
+ // This test is subject to integer overflow but the next is not.
return NULL;
}
- declared_size = GetActualBufferSize(param_count, buffer_base);
- if ((declared_size > buffer_size) ||
- (declared_size < min_declared_size)) {
- // declared size is bigger than buffer or smaller than computed size.
+ actual_size = GetActualBufferSize(param_count, buffer_base);
+ if ((actual_size > buffer_size) || (0 == actual_size)) {
+ // It is too big or too many declared parameters.
return NULL;
}
// Now we copy the actual amount of the message.
- *output_size = declared_size;
- backing_mem = new char[declared_size];
- copied_params = reinterpret_cast<CrossCallParamsEx*>(backing_mem);
- memcpy(backing_mem, call_params, declared_size);
+ actual_size += sizeof(ParamInfo); // To get the last offset.
+ *output_size = actual_size;
+ backing_mem = new char[actual_size];
+ memset(backing_mem, 0, actual_size);
+ // Note that this is a placement new.
+#pragma warning(push)
+#pragma warning(disable: 4291) // No matching operator delete.
+ // TODO(cpu): Remove this warning.
+ copied_params = new(backing_mem)CrossCallParamsEx();
+#pragma warning(pop)
+ memcpy(backing_mem, call_params, actual_size);
} __except(EXCEPTION_EXECUTE_HANDLER) {
// In case of a windows exception we know it occurred while touching the
@@ -140,9 +138,7 @@ CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base,
return NULL;
}
- const char* last_byte = &backing_mem[declared_size - 1];
- const char* first_byte = &backing_mem[min_declared_size];
-
+ char* last_byte = &backing_mem[actual_size - 1];
// Verify here that all and each parameters make sense. This is done in the
// local copy.
for (size_t ix =0; ix != param_count; ++ix) {
@@ -153,7 +149,6 @@ CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base,
if ((NULL == address) || // No null params.
(INVALID_TYPE >= type) || (LAST_TYPE <= type) || // Unknown type.
(address < backing_mem) || // Start cannot point before buffer.
- (address < first_byte) || // Start cannot point too low.
(address > last_byte) || // Start cannot point past buffer.
((address + size) < address) || // Invalid size.
((address + size) > last_byte)) { // End cannot point past buffer.
diff --git a/sandbox/src/ipc_unittest.cc b/sandbox/src/ipc_unittest.cc
index 7686242..909f144 100644
--- a/sandbox/src/ipc_unittest.cc
+++ b/sandbox/src/ipc_unittest.cc
@@ -332,23 +332,6 @@ TEST(IPCTest, CrossCallValidation) {
EXPECT_TRUE(NULL == ccp);
}
#endif // defined(NDEBUG)
-
- ActualCallParams<1, 256> params_3(kTag, 1);
- params_3.CopyParamIn(0, &value, sizeof(value), false, ULONG_TYPE);
- buffer = const_cast<void*>(params_3.GetBuffer());
- EXPECT_TRUE(NULL != buffer);
-
- size_t correct_size = params_3.OverrideSize(1);
- ccp = CrossCallParamsEx::CreateFromBuffer(buffer, 256, &out_size);
- EXPECT_TRUE(NULL == ccp);
-
- params_3.OverrideSize(correct_size - 4);
- ccp = CrossCallParamsEx::CreateFromBuffer(buffer, 256, &out_size);
- EXPECT_TRUE(NULL == ccp);
-
- params_3.OverrideSize(correct_size);
- ccp = CrossCallParamsEx::CreateFromBuffer(buffer, 256, &out_size);
- EXPECT_TRUE(NULL != ccp);
}
// This structure is passed to the mock server threads to simulate