From 4d7cd098001bdd1b25fd3539a76dcf16863e43c1 Mon Sep 17 00:00:00 2001
From: George Burgess IV <gbiv@google.com>
Date: Mon, 11 Jul 2022 12:37:19 -0700
Subject: [PATCH] exploitability: fix buffer overflow

exploitability_linux assumed a 15 byte buffer to always be passed in as
`raw_bytes` for `DisassembleBytes`. This test was passing in a 6 byte
buffer. Make `DisassembleBytes` accept a length.

Bug: b:235999011
Change-Id: I696c66357faa1c7d762c64009864123897f03488
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3756170
Reviewed-by: Mike Frysinger <vapier@chromium.org>
---
 src/processor/exploitability_linux.cc    | 8 +++++---
 src/processor/exploitability_linux.h     | 1 +
 src/processor/exploitability_unittest.cc | 9 ++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/processor/exploitability_linux.cc b/src/processor/exploitability_linux.cc
index 3ec39dd0..7a0fb71a 100644
--- a/src/processor/exploitability_linux.cc
+++ b/src/processor/exploitability_linux.cc
@@ -232,6 +232,7 @@ bool ExploitabilityLinux::EndedOnIllegalWrite(uint64_t instruction_ptr) {
   char objdump_output_buffer[MAX_OBJDUMP_BUFFER_LEN] = {0};
   DisassembleBytes(architecture,
                    raw_memory + offset,
+                   MAX_INSTRUCTION_LEN,
                    MAX_OBJDUMP_BUFFER_LEN,
                    objdump_output_buffer);
 
@@ -483,9 +484,11 @@ bool ExploitabilityLinux::TokenizeObjdumpInstruction(const string& line,
 
 bool ExploitabilityLinux::DisassembleBytes(const string& architecture,
                                            const uint8_t* raw_bytes,
+                                           const unsigned int raw_bytes_len,
                                            const unsigned int buffer_len,
                                            char* objdump_output_buffer) {
-  if (!raw_bytes || !objdump_output_buffer) {
+  if (!raw_bytes || !objdump_output_buffer ||
+      raw_bytes_len > MAX_INSTRUCTION_LEN) {
     BPLOG(ERROR) << "Bad input parameters.";
     return false;
   }
@@ -499,8 +502,7 @@ bool ExploitabilityLinux::DisassembleBytes(const string& architecture,
     unlink(raw_bytes_tmpfile);
     return false;
   }
-  if (write(raw_bytes_fd, raw_bytes, MAX_INSTRUCTION_LEN)
-      != MAX_INSTRUCTION_LEN) {
+  if (write(raw_bytes_fd, raw_bytes, raw_bytes_len) != raw_bytes_len) {
     BPLOG(ERROR) << "Writing of raw bytes failed.";
     unlink(raw_bytes_tmpfile);
     return false;
diff --git a/src/processor/exploitability_linux.h b/src/processor/exploitability_linux.h
index 36647959..84197bb6 100644
--- a/src/processor/exploitability_linux.h
+++ b/src/processor/exploitability_linux.h
@@ -83,6 +83,7 @@ class ExploitabilityLinux : public Exploitability {
   // was a success, and the caller owns all pointers.
   static bool DisassembleBytes(const string& architecture,
                                const uint8_t* raw_bytes,
+                               const unsigned int raw_bytes_len,
                                const unsigned int MAX_OBJDUMP_BUFFER_LEN,
                                char* objdump_output_buffer);
 
diff --git a/src/processor/exploitability_unittest.cc b/src/processor/exploitability_unittest.cc
index 528ee5f2..2baff327 100644
--- a/src/processor/exploitability_unittest.cc
+++ b/src/processor/exploitability_unittest.cc
@@ -30,6 +30,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <type_traits>
 #include <string>
 
 #include "breakpad_googletest_includes.h"
@@ -187,13 +188,11 @@ TEST(ExploitabilityTest, TestLinuxEngine) {
 
 #ifndef _WIN32
 TEST(ExploitabilityLinuxUtilsTest, DisassembleBytesTest) {
-  ASSERT_FALSE(ExploitabilityLinuxTest::DisassembleBytes("", NULL, 5, NULL));
+  ASSERT_FALSE(ExploitabilityLinuxTest::DisassembleBytes("", NULL, 0, 5, NULL));
   uint8_t bytes[6] = {0xc7, 0x0, 0x5, 0x0, 0x0, 0x0};
   char buffer[1024] = {0};
-  ASSERT_TRUE(ExploitabilityLinuxTest::DisassembleBytes("i386:x86-64",
-                                                        bytes,
-                                                        1024,
-                                                        buffer));
+  ASSERT_TRUE(ExploitabilityLinuxTest::DisassembleBytes(
+      "i386:x86-64", bytes, std::extent<decltype(bytes)>::value, 1024, buffer));
   std::stringstream objdump_stream;
   objdump_stream.str(string(buffer));
   string line = "";