From f032e4c3b4eaef64432482cb00eb0c3df33cde48 Mon Sep 17 00:00:00 2001
From: Ivan Penkov <ivanpe@chromium.org>
Date: Wed, 21 Feb 2024 15:38:10 -0800
Subject: [PATCH] Addressing a potential integer underflow in minidump.cc and
 stackwalker_arm64.cc

Also, defining __STDC_FORMAT_MACROS before including <inttypes.h>

Change-Id: Ia25c4353412ca70512efef5e98670687ab575750
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5310977
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
---
 src/processor/minidump.cc           | 8 +++++++-
 src/processor/proc_maps_linux.cc    | 3 ++-
 src/processor/range_map_unittest.cc | 4 ++++
 src/processor/stackwalker_arm64.cc  | 7 ++++++-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc
index 83e5a868..3de36784 100644
--- a/src/processor/minidump.cc
+++ b/src/processor/minidump.cc
@@ -32,6 +32,11 @@
 //
 // Author: Mark Mentovai
 
+// For <inttypes.h> PRI* macros, before anything else might #include it.
+#ifndef __STDC_FORMAT_MACROS
+#define __STDC_FORMAT_MACROS
+#endif  /* __STDC_FORMAT_MACROS */
+
 #ifdef HAVE_CONFIG_H
 #include <config.h>  // Must come first
 #endif
@@ -39,6 +44,7 @@
 #include "google_breakpad/processor/minidump.h"
 
 #include <assert.h>
+#include <cstdint>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <stddef.h>
@@ -820,7 +826,7 @@ bool MinidumpContext::Read(uint32_t expected_size) {
           // Context may include xsave registers and so be larger than
           // sizeof(MDRawContextX86). For now we skip this extended data.
           if (context_flags & MD_CONTEXT_X86_XSTATE) {
-            size_t bytes_left = expected_size - sizeof(MDRawContextX86);
+            int64_t bytes_left = expected_size - sizeof(MDRawContextX86);
             if (bytes_left > kMaxXSaveAreaSize) {
               BPLOG(ERROR) << "MinidumpContext oversized xstate area";
               return false;
diff --git a/src/processor/proc_maps_linux.cc b/src/processor/proc_maps_linux.cc
index 6fcb909a..5234633e 100644
--- a/src/processor/proc_maps_linux.cc
+++ b/src/processor/proc_maps_linux.cc
@@ -2,9 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// For <inttypes.h> PRI* macros, before anything else might #include it.
 #ifndef __STDC_FORMAT_MACROS
 #define __STDC_FORMAT_MACROS
-#endif
+#endif  /* __STDC_FORMAT_MACROS */
 
 #ifdef HAVE_CONFIG_H
 #include <config.h>  // Must come first
diff --git a/src/processor/range_map_unittest.cc b/src/processor/range_map_unittest.cc
index 568df367..f8e3dfbe 100644
--- a/src/processor/range_map_unittest.cc
+++ b/src/processor/range_map_unittest.cc
@@ -30,6 +30,10 @@
 //
 // Author: Mark Mentovai
 
+// For <inttypes.h> PRI* macros, before anything else might #include it.
+#ifndef __STDC_FORMAT_MACROS
+#define __STDC_FORMAT_MACROS
+#endif  /* __STDC_FORMAT_MACROS */
 
 #ifdef HAVE_CONFIG_H
 #include <config.h>  // Must come first
diff --git a/src/processor/stackwalker_arm64.cc b/src/processor/stackwalker_arm64.cc
index 0fa02e04..8ab22ba9 100644
--- a/src/processor/stackwalker_arm64.cc
+++ b/src/processor/stackwalker_arm64.cc
@@ -36,6 +36,7 @@
 #include <config.h>  // Must come first
 #endif
 
+#include <cstdint>
 #include <vector>
 
 #include "common/scoped_ptr.h"
@@ -269,11 +270,15 @@ void StackwalkerARM64::CorrectRegLRByFramePointer(
 
   // Searching for a real callee frame. Skipping inline frames since they
   // don't contain context (and cannot be downcasted to StackFrameARM64).
-  size_t last_frame_callee_id = frames.size() - 2;
+  int64_t last_frame_callee_id = frames.size() - 2;
   while (last_frame_callee_id >= 0 && frames[last_frame_callee_id]->trust ==
                                           StackFrame::FRAME_TRUST_INLINE) {
     last_frame_callee_id--;
   }
+  // last_frame_callee_id should not become negative because at the top of the
+  // stack trace we always have a context frame (FRAME_TRUST_CONTEXT) so the
+  // above loop should end before last_frame_callee_id gets negative. But we are
+  // being extra defensive here and bail if it ever becomes negative.
   if (last_frame_callee_id < 0) return;
   StackFrameARM64* last_frame_callee =
       static_cast<StackFrameARM64*>(frames[last_frame_callee_id]);