From 8205b6edb87f74fa1f6cfb861daa5da55253d0ad Mon Sep 17 00:00:00 2001 From: Ivan Penkov Date: Mon, 31 Jan 2022 19:07:58 +0000 Subject: [PATCH] The X86 stack walker was doing an illegal down cast from base-class (StackFrame) to derived-class (StackFrameX86). Inline frames are always of the base-class type (StackFrame). Treating them as derived-class and accessing members was causing heap buffer overflows. Change-Id: Id4122ab6a31f016933038a1cb63d45d5c38481f5 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3425445 Reviewed-by: Joshua Peraza --- src/processor/stackwalker_x86.cc | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index b11e061d..c11a04d5 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -141,6 +141,9 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo( bool stack_scan_allowed) { StackFrame::FrameTrust trust = StackFrame::FRAME_TRUST_NONE; + // The last frame can never be inline. A sequence of inline frames always + // finishes with a conventional frame. + assert(frames.back()->trust != StackFrame::FRAME_TRUST_INLINE); StackFrameX86* last_frame = static_cast(frames.back()); // Save the stack walking info we found, in case we need it later to @@ -187,9 +190,15 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo( uint32_t last_frame_callee_parameter_size = 0; int frames_already_walked = frames.size(); - if (frames_already_walked >= 2) { + for (int last_frame_callee_id = frames_already_walked - 2; + last_frame_callee_id >= 0; last_frame_callee_id--) { + // Searching for a real callee frame. Skipping inline frames since they + // cannot be downcasted to StackFrameX86. + if (frames[last_frame_callee_id]->trust == StackFrame::FRAME_TRUST_INLINE) { + continue; + } const StackFrameX86* last_frame_callee - = static_cast(frames[frames_already_walked - 2]); + = static_cast(frames[last_frame_callee_id]); WindowsFrameInfo* last_frame_callee_info = last_frame_callee->windows_frame_info; if (last_frame_callee_info && @@ -516,6 +525,9 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo( StackFrameX86* StackwalkerX86::GetCallerByCFIFrameInfo( const vector& frames, CFIFrameInfo* cfi_frame_info) { + // The last frame can never be inline. A sequence of inline frames always + // finishes with a conventional frame. + assert(frames.back()->trust != StackFrame::FRAME_TRUST_INLINE); StackFrameX86* last_frame = static_cast(frames.back()); last_frame->cfi_frame_info = cfi_frame_info; @@ -542,6 +554,9 @@ StackFrameX86* StackwalkerX86::GetCallerByEBPAtBase( const vector& frames, bool stack_scan_allowed) { StackFrame::FrameTrust trust; + // The last frame can never be inline. A sequence of inline frames always + // finishes with a conventional frame. + assert(frames.back()->trust != StackFrame::FRAME_TRUST_INLINE); StackFrameX86* last_frame = static_cast(frames.back()); uint32_t last_esp = last_frame->context.esp; uint32_t last_ebp = last_frame->context.ebp; @@ -634,6 +649,9 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack* stack, const vector& frames = *stack->frames(); StackFrameX86* last_frame = static_cast(frames.back()); + // The last frame can never be inline. A sequence of inline frames always + // finishes with a conventional frame. + assert(last_frame->trust != StackFrame::FRAME_TRUST_INLINE); scoped_ptr new_frame; // If the resolver has Windows stack walking information, use that.