Index: sandbox/src/crosscall_server.cc |
=================================================================== |
--- sandbox/src/crosscall_server.cc (revision 56798) |
+++ sandbox/src/crosscall_server.cc (working copy) |
@@ -85,6 +85,8 @@ |
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; |
} |
@@ -94,10 +96,12 @@ |
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. |
@@ -107,38 +111,43 @@ |
// Check against the minimum size given the number of stated params |
// if too small we bail out. |
param_count = call_params->GetParamsCount(); |
- if ((buffer_size - sizeof(CrossCallParams)) < |
- (sizeof(ptrdiff_t) * (param_count + 1))) { |
- // This test is subject to integer overflow but the next is not. |
+ |
+ min_declared_size = |
+ sizeof(CrossCallParamsEx) + (param_count * sizeof(ParamInfo)); |
+ |
+ if ((buffer_size < min_declared_size) || |
+ (sizeof(CrossCallParamsEx) > min_declared_size)) { |
+ // Minimal computed size bigger than existing buffer or param_count |
+ // integer overflow. |
return NULL; |
} |
- actual_size = GetActualBufferSize(param_count, buffer_base); |
- if ((actual_size > buffer_size) || (0 == actual_size)) { |
- // It is too big or too many declared parameters. |
+ // Retrieve the declared size which if it fails returns 0. |
+ 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 |
+ // or param_count 0 or bigger than 9. |
return NULL; |
} |
// Now we copy the actual amount of the message. |
- 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); |
+ *output_size = declared_size; |
+ backing_mem = new char[declared_size]; |
+ copied_params = reinterpret_cast<CrossCallParamsEx*>(backing_mem); |
+ memcpy(backing_mem, call_params, declared_size); |
} __except(EXCEPTION_EXECUTE_HANDLER) { |
// In case of a windows exception we know it occurred while touching the |
// untrusted buffer so we bail out as is. |
+ delete [] backing_mem; |
return NULL; |
} |
- char* last_byte = &backing_mem[actual_size - 1]; |
+ const char* last_byte = &backing_mem[declared_size]; |
+ const char* first_byte = &backing_mem[min_declared_size]; |
+ |
// 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) { |
@@ -149,6 +158,7 @@ |
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. |
@@ -164,7 +174,7 @@ |
// Accessors to the parameters in the raw buffer. |
void* CrossCallParamsEx::GetRawParameter(size_t index, size_t* size, |
ArgType* type) { |
- if (index > GetParamsCount()) { |
+ if (index >= GetParamsCount()) { |
return NULL; |
} |
// The size is always computed from the parameter minus the next |