win: Only process up to EXCEPTION_MAXIMUM_PARAMETERS in an EXCEPTION_RECORD

The EXCEPTION_RECORD contains a NumberParameters field, which could
store a value that exceeds the amount of space allocated for the
ExceptionInformation array.

Bug: chromium:1412658
Change-Id: Ibfed8eb6317e28d3addf9215cda7fffc32e1030d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4284559
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
This commit is contained in:
Robert Sesek 2023-02-22 18:37:10 -05:00 committed by Crashpad LUCI CQ
parent 70e0f92153
commit 3e8727238b
2 changed files with 52 additions and 1 deletions

View File

@ -14,6 +14,8 @@
#include "snapshot/win/exception_snapshot_win.h"
#include <algorithm>
#include "base/logging.h"
#include "snapshot/capture_memory.h"
#include "snapshot/memory_snapshot.h"
@ -261,8 +263,12 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
exception_code_ = first_record.ExceptionCode;
exception_flags_ = first_record.ExceptionFlags;
exception_address_ = first_record.ExceptionAddress;
for (DWORD i = 0; i < first_record.NumberParameters; ++i)
const DWORD number_parameters = std::min<DWORD>(
first_record.NumberParameters, EXCEPTION_MAXIMUM_PARAMETERS);
for (DWORD i = 0; i < number_parameters; ++i) {
codes_.push_back(first_record.ExceptionInformation[i]);
}
if (first_record.ExceptionRecord) {
// https://crashpad.chromium.org/bug/43
LOG(WARNING) << "dropping chained ExceptionRecord";

View File

@ -14,11 +14,14 @@
#include "snapshot/win/exception_snapshot_win.h"
#include <windows.h>
#include <string>
#include "base/files/file_path.h"
#include "base/strings/utf_string_conversions.h"
#include "gtest/gtest.h"
#include "snapshot/win/exception_snapshot_win.h"
#include "snapshot/win/process_snapshot_win.h"
#include "test/errors.h"
#include "test/test_paths.h"
@ -315,6 +318,48 @@ TEST(SimulateCrash, ChildDumpWithoutCrashingWOW64) {
}
#endif // ARCH_CPU_64_BITS
TEST(ExceptionSnapshot, TooManyExceptionParameters) {
ProcessReaderWin process_reader;
ASSERT_TRUE(process_reader.Initialize(GetCurrentProcess(),
ProcessSuspensionState::kRunning));
// Construct a fake exception record and CPU context.
auto exception_record = std::make_unique<EXCEPTION_RECORD>();
exception_record->ExceptionCode = STATUS_FATAL_APP_EXIT;
exception_record->ExceptionFlags = EXCEPTION_NONCONTINUABLE;
exception_record->ExceptionAddress = reinterpret_cast<PVOID>(0xFA15E);
// One more than is permitted in the struct.
exception_record->NumberParameters = EXCEPTION_MAXIMUM_PARAMETERS + 1;
for (int i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; ++i) {
exception_record->ExceptionInformation[i] = 1000 + i;
}
auto cpu_context = std::make_unique<internal::CPUContextUnion>();
auto exception_pointers = std::make_unique<EXCEPTION_POINTERS>();
exception_pointers->ExceptionRecord =
reinterpret_cast<PEXCEPTION_RECORD>(exception_record.get());
exception_pointers->ContextRecord =
reinterpret_cast<PCONTEXT>(cpu_context.get());
internal::ExceptionSnapshotWin snapshot;
ASSERT_TRUE(snapshot.Initialize(
&process_reader,
GetCurrentThreadId(),
reinterpret_cast<WinVMAddress>(exception_pointers.get()),
nullptr));
EXPECT_EQ(STATUS_FATAL_APP_EXIT, snapshot.Exception());
EXPECT_EQ(static_cast<uint32_t>(EXCEPTION_NONCONTINUABLE),
snapshot.ExceptionInfo());
EXPECT_EQ(0xFA15Eu, snapshot.ExceptionAddress());
EXPECT_EQ(static_cast<size_t>(EXCEPTION_MAXIMUM_PARAMETERS),
snapshot.Codes().size());
for (size_t i = 0; i < EXCEPTION_MAXIMUM_PARAMETERS; ++i) {
EXPECT_EQ(1000 + i, snapshot.Codes()[i]);
}
}
} // namespace
} // namespace test
} // namespace crashpad