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>
This commit is contained in:
George Burgess IV 2022-07-11 12:37:19 -07:00 committed by George Burgess
parent c161459d7e
commit 4d7cd09800
3 changed files with 10 additions and 8 deletions

View file

@ -232,6 +232,7 @@ bool ExploitabilityLinux::EndedOnIllegalWrite(uint64_t instruction_ptr) {
char objdump_output_buffer[MAX_OBJDUMP_BUFFER_LEN] = {0}; char objdump_output_buffer[MAX_OBJDUMP_BUFFER_LEN] = {0};
DisassembleBytes(architecture, DisassembleBytes(architecture,
raw_memory + offset, raw_memory + offset,
MAX_INSTRUCTION_LEN,
MAX_OBJDUMP_BUFFER_LEN, MAX_OBJDUMP_BUFFER_LEN,
objdump_output_buffer); objdump_output_buffer);
@ -483,9 +484,11 @@ bool ExploitabilityLinux::TokenizeObjdumpInstruction(const string& line,
bool ExploitabilityLinux::DisassembleBytes(const string& architecture, bool ExploitabilityLinux::DisassembleBytes(const string& architecture,
const uint8_t* raw_bytes, const uint8_t* raw_bytes,
const unsigned int raw_bytes_len,
const unsigned int buffer_len, const unsigned int buffer_len,
char* objdump_output_buffer) { 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."; BPLOG(ERROR) << "Bad input parameters.";
return false; return false;
} }
@ -499,8 +502,7 @@ bool ExploitabilityLinux::DisassembleBytes(const string& architecture,
unlink(raw_bytes_tmpfile); unlink(raw_bytes_tmpfile);
return false; return false;
} }
if (write(raw_bytes_fd, raw_bytes, MAX_INSTRUCTION_LEN) if (write(raw_bytes_fd, raw_bytes, raw_bytes_len) != raw_bytes_len) {
!= MAX_INSTRUCTION_LEN) {
BPLOG(ERROR) << "Writing of raw bytes failed."; BPLOG(ERROR) << "Writing of raw bytes failed.";
unlink(raw_bytes_tmpfile); unlink(raw_bytes_tmpfile);
return false; return false;

View file

@ -83,6 +83,7 @@ class ExploitabilityLinux : public Exploitability {
// was a success, and the caller owns all pointers. // was a success, and the caller owns all pointers.
static bool DisassembleBytes(const string& architecture, static bool DisassembleBytes(const string& architecture,
const uint8_t* raw_bytes, const uint8_t* raw_bytes,
const unsigned int raw_bytes_len,
const unsigned int MAX_OBJDUMP_BUFFER_LEN, const unsigned int MAX_OBJDUMP_BUFFER_LEN,
char* objdump_output_buffer); char* objdump_output_buffer);

View file

@ -30,6 +30,7 @@
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h> #include <unistd.h>
#include <type_traits>
#include <string> #include <string>
#include "breakpad_googletest_includes.h" #include "breakpad_googletest_includes.h"
@ -187,13 +188,11 @@ TEST(ExploitabilityTest, TestLinuxEngine) {
#ifndef _WIN32 #ifndef _WIN32
TEST(ExploitabilityLinuxUtilsTest, DisassembleBytesTest) { 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}; uint8_t bytes[6] = {0xc7, 0x0, 0x5, 0x0, 0x0, 0x0};
char buffer[1024] = {0}; char buffer[1024] = {0};
ASSERT_TRUE(ExploitabilityLinuxTest::DisassembleBytes("i386:x86-64", ASSERT_TRUE(ExploitabilityLinuxTest::DisassembleBytes(
bytes, "i386:x86-64", bytes, std::extent<decltype(bytes)>::value, 1024, buffer));
1024,
buffer));
std::stringstream objdump_stream; std::stringstream objdump_stream;
objdump_stream.str(string(buffer)); objdump_stream.str(string(buffer));
string line = ""; string line = "";