Index: sandbox/src/crosscall_server.cc |
=================================================================== |
--- sandbox/src/crosscall_server.cc (revision 56797) |
+++ sandbox/src/crosscall_server.cc (working copy) |
@@ -85,8 +85,6 @@ |
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 @@ |
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 @@ |
// 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 @@ |
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 @@ |
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. |