commit a64a7c2de648ad87f392e407a92589515a72b0ef Author: Tom Ritter tom@mozilla.com Date: Thu Jun 7 13:08:27 2018 -0500

Bug 1461421 Use OffsetOf to calculate the location of param_info_ rather than assuming it's at the end of the parent class r?bobowen

MozReview-Commit-ID: D7REZiAIMpN --- .../chromium/sandbox/win/src/crosscall_params.h | 5 ++ .../chromium/sandbox/win/src/crosscall_server.cc | 56 +++++++++++++++++----- 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/security/sandbox/chromium/sandbox/win/src/crosscall_params.h b/security/sandbox/chromium/sandbox/win/src/crosscall_params.h index eb59c44239e2..baceebd9e285 100644 --- a/security/sandbox/chromium/sandbox/win/src/crosscall_params.h +++ b/security/sandbox/chromium/sandbox/win/src/crosscall_params.h @@ -60,6 +60,7 @@ union MultiType { // - Add another Callback typedef to Dispatcher. // - Add another case to the switch on SharedMemIPCServer::InvokeCallback. // - Add another case to the switch in GetActualAndMaxBufferSize +// - Add another case to the switch in GetOffsetOfFirstMemberOfActualCallParams const int kMaxIpcParams = 9;

// Contains the information about a parameter in the ipc buffer. @@ -92,6 +93,8 @@ struct CrossCallReturn { MultiType extended[kExtendedReturnCount]; };

+uint32_t GetOffsetOfFirstMemberOfActualCallParams(uint32_t param_count); + // CrossCallParams base class that models the input params all packed in a // single compact memory blob. The representation can vary but in general a // given child of this class is meant to represent all input parameters @@ -276,6 +279,8 @@ class ActualCallParams : public CrossCallParams { char parameters_[BLOCK_SIZE - sizeof(CrossCallParams) - sizeof(ParamInfo) * (NUMBER_PARAMS + 1)]; DISALLOW_COPY_AND_ASSIGN(ActualCallParams); + + friend uint32_t GetOffsetOfFirstMemberOfActualCallParams(uint32_t param_count); };

static_assert(sizeof(ActualCallParams<1, 1024>) == 1024, "bad size buffer"); diff --git a/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc b/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc index 9f71f333f02d..6d94b1cb14c2 100644 --- a/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc +++ b/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc @@ -27,20 +27,21 @@ const size_t kMaxBufferSize = sandbox::kIPCChannelSize;

namespace sandbox {

+// The template types are used to calculate the maximum expected size. +typedef ActualCallParams<0, kMaxBufferSize> ActualCP0; +typedef ActualCallParams<1, kMaxBufferSize> ActualCP1; +typedef ActualCallParams<2, kMaxBufferSize> ActualCP2; +typedef ActualCallParams<3, kMaxBufferSize> ActualCP3; +typedef ActualCallParams<4, kMaxBufferSize> ActualCP4; +typedef ActualCallParams<5, kMaxBufferSize> ActualCP5; +typedef ActualCallParams<6, kMaxBufferSize> ActualCP6; +typedef ActualCallParams<7, kMaxBufferSize> ActualCP7; +typedef ActualCallParams<8, kMaxBufferSize> ActualCP8; +typedef ActualCallParams<9, kMaxBufferSize> ActualCP9; + // Returns the actual size for the parameters in an IPC buffer. Returns // zero if the |param_count| is zero or too big. uint32_t GetActualBufferSize(uint32_t param_count, void* buffer_base) { - // The template types are used to calculate the maximum expected size. - typedef ActualCallParams<1, kMaxBufferSize> ActualCP1; - typedef ActualCallParams<2, kMaxBufferSize> ActualCP2; - typedef ActualCallParams<3, kMaxBufferSize> ActualCP3; - typedef ActualCallParams<4, kMaxBufferSize> ActualCP4; - typedef ActualCallParams<5, kMaxBufferSize> ActualCP5; - typedef ActualCallParams<6, kMaxBufferSize> ActualCP6; - typedef ActualCallParams<7, kMaxBufferSize> ActualCP7; - typedef ActualCallParams<8, kMaxBufferSize> ActualCP8; - typedef ActualCallParams<9, kMaxBufferSize> ActualCP9; - // Retrieve the actual size and the maximum size of the params buffer. switch (param_count) { case 0: @@ -68,6 +69,35 @@ uint32_t GetActualBufferSize(uint32_t param_count, void* buffer_base) { } }

+// Returns the actual size for the parameters in an IPC buffer. Returns +// zero if the |param_count| is zero or too big. +uint32_t GetOffsetOfFirstMemberOfActualCallParams(uint32_t param_count) { + switch (param_count) { + case 0: + return offsetof(ActualCP0, param_info_); + case 1: + return offsetof(ActualCP1, param_info_); + case 2: + return offsetof(ActualCP2, param_info_); + case 3: + return offsetof(ActualCP3, param_info_); + case 4: + return offsetof(ActualCP4, param_info_); + case 5: + return offsetof(ActualCP5, param_info_); + case 6: + return offsetof(ActualCP6, param_info_); + case 7: + return offsetof(ActualCP7, param_info_); + case 8: + return offsetof(ActualCP8, param_info_); + case 9: + return offsetof(ActualCP9, param_info_); + default: + return 0; + } +} + // Verifies that the declared sizes of an IPC buffer are within range. bool IsSizeWithinRange(uint32_t buffer_size, uint32_t min_declared_size, @@ -137,7 +167,7 @@ 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(CrossCallParams) + + min_declared_size = GetOffsetOfFirstMemberOfActualCallParams(param_count) + ((param_count + 1) * sizeof(ParamInfo));

// Retrieve the declared size which if it fails returns 0. @@ -157,7 +187,7 @@ CrossCallParamsEx* CrossCallParamsEx::CreateFromBuffer(void* buffer_base, // should be actually read. _ReadWriteBarrier();

- min_declared_size = sizeof(CrossCallParams) + + min_declared_size = GetOffsetOfFirstMemberOfActualCallParams(param_count) + ((param_count + 1) * sizeof(ParamInfo));

// Check that the copied buffer is still valid.