Add check for Linux minidump ending on bad write for exploitability rating.

src/processor/exploitability_linux.cc

Index: src/processor/exploitability_linux.cc
===================================================================
--- src/processor/exploitability_linux.cc	(revision 1476)
+++ src/processor/exploitability_linux.cc	(working copy)
@@ -36,6 +36,16 @@
 
 #include "processor/exploitability_linux.h"
 
+#include <regex.h>
+#include <stdio.h>
+#include <string.h>
+#include <sysexits.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <sstream>
+#include <iterator>
+
 #include "google_breakpad/common/minidump_exception_linux.h"
 #include "google_breakpad/processor/call_stack.h"
 #include "google_breakpad/processor/process_state.h"
@@ -42,6 +52,10 @@
 #include "google_breakpad/processor/stack_frame.h"
 #include "processor/logging.h"
 
+#define MAX_INSTRUCTION_LEN 15
+// This is the buffer size for objdump's output.
+#define MAX_OBJDUMP_BUFFER_LEN 4096
+
 namespace {
 
 // This function in libc is called if the program was compiled with
@@ -115,8 +129,10 @@
     return EXPLOITABILITY_ERR_PROCESSING;
   }
 
-  // Checking for the instruction pointer in a valid instruction region.
-  if (!this->InstructionPointerInCode(instruction_ptr)) {
+  // Checking for the instruction pointer in a valid instruction region
+  // or if the crash resulted during an illegal write.
+  if (!this->InstructionPointerInCode(instruction_ptr) ||
+       this->EndedOnIllegalWrite(instruction_ptr)) {
     return EXPLOITABILITY_HIGH;
   }
 
@@ -125,6 +141,305 @@
   return EXPLOITABILITY_INTERESTING;
 }
 
+bool ExploitabilityLinux::EndedOnIllegalWrite(uint64_t instruction_ptr) {
+  // Get memory region containing instruction pointer.
+  MinidumpMemoryList *memory_list = dump_->GetMemoryList();
+  MinidumpMemoryRegion *memory_region =
+      memory_list ?
+      memory_list->GetMemoryRegionForAddress(instruction_ptr) : NULL;
+  if (memory_region == NULL) {
+    BPLOG(ERROR) << "No memory region around instruction pointer.";
+    return false;
+  }
+
+  // Get exception data to find architecture.
+  string architecture = "";
+  MinidumpException *exception = dump_->GetException();
+  // This should never evaluate to true, since this should not be reachable
+  // without checking for exception data earlier.
+  if (exception == NULL) {
+    BPLOG(INFO) << "No exception data.";
+    return 0;

[ivanpe, 2015/08/10 19:58:04]

The return value type is bool

[liuandrew, 2015/08/10 22:00:40]

Done.

+  }
+  const MDRawExceptionStream *raw_exception_stream = exception->exception();
+  const MinidumpContext *context = exception->GetContext();
+  // This should not evaluate to true, for the same reason mentioned above.
+  if (raw_exception_stream == NULL || context == NULL) {
+    BPLOG(INFO) << "No exception or architecture data.";
+    return false;
+  }
+  // Check architecture and set architecture variable to corresponding flag
+  // in objdump.
+  switch (context->GetContextCPU()) {
+    case MD_CONTEXT_X86:
+      architecture = "i386";
+      break;
+    case MD_CONTEXT_AMD64:
+      architecture = "i386:x86-64";
+      break;
+    default:
+      // Unsupported architecture. Note that ARM architectures are not
+      // supported because objdump does not support ARM.
+      return false;
+      break;
+  }
+
+  // Get memory region around instruction pointer and the number of bytes
+  // before and after the instruction pointer in the memory region.
+  const uint8_t *raw_memory = memory_region->GetMemory();
+  const uint32_t offset = instruction_ptr - memory_region->GetBase();

[ivanpe, 2015/08/10 19:58:04]

Both instruction_ptr and memory_region->GetBase() are 64-bit uints. I think
offset should also be 64-bit value.  Also, you never check whether
memory_region->GetBase() <= instruction_ptr and since the underlying type
(uint64) does not support negative numbers you may end up an integer overflow.

[liuandrew, 2015/08/10 22:00:40]

Done.

+  if (memory_region->GetSize() - offset < MAX_INSTRUCTION_LEN) {
+    BPLOG(ERROR) << "Not enough bytes left to guarantee complete instruction";
+    return false;
+  }
+
+  // Write raw bytes around instruction pointer to a temporary file to
+  // pass as an argument to objdump.
+  char raw_bytes_tmpfile[] = "/tmp/breakpad_mem_region-raw_bytes-XXXXXX";

[ivanpe, 2015/08/10 19:58:04]

const char?

[liuandrew, 2015/08/10 22:00:40]

No, it should not be const. Calling mkstemp modifies this variable, and the
result is the name of the file. It is fine to allocate this on the stack because
the method changes the "X's" to different characters, so the length of the
string stays the same.

+  int raw_bytes_fd = mkstemp(raw_bytes_tmpfile);

[ivanpe, 2015/08/10 19:58:04]

Please, check return value

[liuandrew, 2015/08/10 22:00:40]

Done.

+  if (write(raw_bytes_fd, raw_memory + offset, MAX_INSTRUCTION_LEN)
+      != MAX_INSTRUCTION_LEN) {
+    BPLOG(ERROR) << "Writing of raw bytes failed.";
+  }
+
+  char objdump_output_buffer[MAX_OBJDUMP_BUFFER_LEN];
+  int pipe_fd[2];
+
+  // Open pipe between STDOUT and the objdump output buffer.
+  if (pipe(pipe_fd) < 0) {
+    BPLOG(ERROR) << "Failed to pipe.";
+    unlink(raw_bytes_tmpfile);
+    return false;
+  }
+
+  // Fork process to call objdump.
+  pid_t child_pid;
+  if ((child_pid = fork()) < 0) {
+    BPLOG(ERROR) << "Forking failed.";
+    unlink(raw_bytes_tmpfile);
+    return false;
+  }
+
+  if (child_pid) {  // Parent code.
+    close(pipe_fd[1]);
+    // Read piped output from objdump.
+    ssize_t bytes_read = read(pipe_fd[0],
+                              objdump_output_buffer,
+                              MAX_OBJDUMP_BUFFER_LEN);
+    wait(NULL);  // Wait for child process to run objdump.
+    unlink(raw_bytes_tmpfile);
+    if (bytes_read < 0) {
+      BPLOG(ERROR) << "Failed to read objdump output.";
+      return false;
+    }
+    close(pipe_fd[0]);
+  } else {  // Child code.
+    close(pipe_fd[0]);
+    dup2(pipe_fd[1], STDOUT_FILENO);  // Send objdump output across pipe.
+    // Exec objdump.
+    execlp("objdump", "objdump", "-D", "-b", "binary", "-M", "intel",

[ivanpe, 2015/08/10 19:58:04]

This function seems to have lots of portability issues, and I'm not sure this is
a such great idea.

 - This makes breakpad processor dependent on an external tool (objdump).
 - C fork and pipe are not very portable as well.

Isn't it possible to use a dis-assembler though a portable library?

[liuandrew, 2015/08/10 22:00:40]

There looked to be no good disassembler libraries to use. Beaengine had a
restrictive license, and Capstone had security issues. Libdisasm only supports
32-bit architectures.

Since objdump is exec'd in a child process, it dies gracefully if any
portability issues arise. Why would fork and pipe have a portability problem?

[ivanpe, 2015/08/10 23:27:58]

Fork and pipe will have portability issues if breakpad processor is run on a
non-unix based OS (I know some people were trying to run it on Windows).  I'm
also worried about performance.  We are starting a new process for each crash
report.  What if objdump hangs?  We should set a timeout so that if the external
process does not respond in, say, a half second, we bail and kill it.  Also, I'm
not sure whether this will work on Borg.

[liuandrew, 2015/08/11 22:55:38]

I tested objdump, and it works in borg.

If I set a timeout to kill the child process, would it be ok to use this?

[ivanpe, 2015/08/11 23:29:43]

I guess that if someone wants to compile this on a platform that does not
support pipe, fork, and objdump, he could solve this by conditional compilation.

Yes, I think we definitely need to have a timeout here.  If for whatever reasons
objdump hangs, the processor must recover and continue processing reports.

[liuandrew, 2015/08/17 21:37:36]

I set an alarm to kill the child process after one second.

+           "-m", architecture.c_str(), raw_bytes_tmpfile, NULL);
+    BPLOG(ERROR) << "Exec failed.";
+    exit(EX_OSERR);
+  }
+
+  // Set up regular expression to catch first instruction from objdump.
+  // The line with the instruction will begin with "0:".
+  regex_t regex;
+  regcomp(&regex, "0:", REG_EXTENDED | REG_NOSUB);
+
+  // Put buffer data into stream to output line-by-line.
+  std::stringstream objdump_stream;
+  objdump_stream.str(string(objdump_output_buffer));
+  string line;
+
+  // Pipe each output line into the string until the string contains
+  // the first instruction from objdump.
+  do {
+    if (!getline(objdump_stream, line)) {
+      BPLOG(ERROR) << "Objdump instructions not found";
+      return false;
+    }
+  } while (regexec(&regex, line.c_str(), 0, NULL, 0));
+  regfree(&regex);  // Free regex data.
+
+  // Tokenize the objdump line.
+  vector<string> tokens;
+  std::istringstream line_stream(line);
+  copy(std::istream_iterator<string>(line_stream),
+       std::istream_iterator<string>(),
+       std::back_inserter(tokens));
+
+  // Parse out the operator and operands from the instruction.
+  string instruction = "";
+  string operands = "";
+
+  // Regex for the data in hex form. Each byte is two hex digits.
+  regcomp(&regex, "^[[:xdigit:]]{2}$", REG_EXTENDED | REG_NOSUB);
+
+  // Find and set the location of the operator. The operator appears
+  // directly after the chain of bytes that define the instruction. The
+  // operands will be the last token, given that the instruction has operands.
+  // If not, the operator is the last token. The loop skips the first token
+  // because the first token is the instruction number (namely "0:").
+  for (size_t i = 1; i < tokens.size(); i++) {
+    // Check if current token no longer is in byte format.
+    if (regexec(&regex, tokens[i].c_str(), 0, NULL, 0)) {
+      instruction = tokens[i];
+      // If the operator is the last token, there are no operands.
+      if (i != tokens.size() - 1) {
+        operands = tokens[tokens.size() - 1];
+      }
+      break;
+    }
+  }
+  regfree(&regex);
+
+  if (instruction.empty()) {
+    BPLOG(ERROR) << "Failed to parse out operation from objdump instruction.";
+    return false;
+  }
+
+  // Split operands into source and destination (if applicable).
+  string dest = "";
+  string src = "";
+  if (!operands.empty()) {
+    size_t delim = operands.find(',');
+    if (delim == string::npos) {
+      dest = operands;
+    } else {
+      dest = operands.substr(0, delim);
+      src = operands.substr(delim + 1);
+    }
+  }
+
+  // Check if the operation is a write to memory. First, the instruction
+  // must one that can write to memory. Second, the write destination
+  // must be a spot in memory rather than a register. Since there are no
+  // symbols from objdump, the destination will be enclosed by brackets.
+  if (dest.at(0) == '[' && dest.at(dest.size() - 1) == ']' &&
+      (!instruction.compare("mov") || !instruction.compare("inc") ||
+       !instruction.compare("dec") || !instruction.compare("and") ||
+       !instruction.compare("or") || !instruction.compare("xor") ||
+       !instruction.compare("not") || !instruction.compare("neg") ||
+       !instruction.compare("add") || !instruction.compare("sub") ||
+       !instruction.compare("shl") || !instruction.compare("shr"))) {
+    // Strip away enclosing brackets from the destination address.
+    dest = dest.substr(1, dest.size() - 2);
+
+    // The destination should be the format [reg+a] or [reg-a], where reg
+    // is a register and a is a hexadecimal constant. Although more complex
+    // expressions can make valid instructions, objdump's disassembly outputs
+    // it in this simpler format.
+
+    // Parse out the constant that is added to the address (if it exists).
+    size_t delim = dest.find('+');
+    bool positive_add_constant = true;
+    // Check if constant is subtracted instead of added.
+    if (delim == string::npos) {
+      positive_add_constant = false;
+      delim = dest.find('-');
+    }
+    uint32_t add_constant = 0;
+    // Save constant and remove it from the expression.
+    if (delim != string::npos) {
+      sscanf(dest.substr(delim + 1).c_str(), "%x", &add_constant);
+      dest = dest.substr(0, delim);
+    }
+
+    // Calculate and set the address that is the target of the write operation.
+    uint64_t write_address = 0;
+
+    // Set the the write address to the corresponding register.
+    // TODO(liuandrew): Add support for partial registers, such as
+    // the rax/eax/ax/ah/al chain.
+    switch (context->GetContextCPU()) {
+      case MD_CONTEXT_X86:
+        if (!dest.compare("eax")) {
+            write_address = context->GetContextX86()->eax;
+        } else if (!dest.compare("ebx")) {
+            write_address = context->GetContextX86()->ebx;
+        } else if (!dest.compare("ecx")) {
+            write_address = context->GetContextX86()->ecx;
+        } else if (!dest.compare("edx")) {
+            write_address = context->GetContextX86()->edx;
+        } else if (!dest.compare("edi")) {
+            write_address = context->GetContextX86()->edi;
+        } else if (!dest.compare("esi")) {
+            write_address = context->GetContextX86()->esi;
+        } else if (!dest.compare("ebp")) {
+            write_address = context->GetContextX86()->ebp;
+        } else if (!dest.compare("esp")) {
+            write_address = context->GetContextX86()->esp;
+        } else if (!dest.compare("eip")) {
+            write_address = context->GetContextX86()->eip;
+        } else {
+          BPLOG(ERROR) << "Unsupported register";
+          return false;
+        }
+        break;
+      case MD_CONTEXT_AMD64:
+        if (!dest.compare("rax")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rbx")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rcx")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rdx")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rdi")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rsi")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rbp")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rsp")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("rip")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r8")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r9")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r10")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r11")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r12")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r13")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r14")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else if (!dest.compare("r15")) {
+          write_address = context->GetContextAMD64()->rax;
+        } else {
+          BPLOG(ERROR) << "Unsupported register";
+          return false;
+        }
+        break;
+      default:
+        // This should not occur since the same switch condition
+        // should have terminated this method.
+        return false;
+        break;
+    }
+
+    // Add or subtract constant from write address (if applicable).
+    write_address =
+        positive_add_constant ?
+        write_address + add_constant : write_address - add_constant;
+
+    // If the program crashed as a result of a write, the destination of
+    // the write must have been an address that did not permit writing.
+    // However, if the address is under 4k, due to program protections,
+    // the crash does not suggest exploitability for writes with such a
+    // low target address.
+    return write_address > 4096;
+  }
+  return false;
+}
+
 bool ExploitabilityLinux::InstructionPointerInCode(uint64_t instruction_ptr) {
   // Get Linux memory mapping from /proc/self/maps. Checking whether the
   // region the instruction pointer is in has executable permission can tell