From bb8e777557ddbdeabdedea4f23613c5021ffd7b1 Mon Sep 17 00:00:00 2001
From: Paul Wankadia <junyer@google.com>
Date: Mon, 25 Nov 2019 03:21:08 -0800
Subject: [PATCH] Fix nullptr-with-nonzero-offset bugs found by UBSan.

Change-Id: Ic94c887b3ba4211bec5f7eb096f06fe5bca53b70
Reviewed-on: https://code-review.googlesource.com/c/re2/+/48852
Reviewed-by: Paul Wankadia <junyer@google.com>
---
 re2/bitstate.cc          |  4 ++++
 re2/nfa.cc               | 23 ++++++++++++++++++++---
 re2/testing/backtrack.cc |  4 ++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/re2/bitstate.cc b/re2/bitstate.cc
index 5cbc0709b..f15c1e43f 100644
--- a/re2/bitstate.cc
+++ b/re2/bitstate.cc
@@ -342,6 +342,10 @@ bool BitState::Search(const StringPiece& text, const StringPiece& context,
     cap_[0] = p;
     if (TrySearch(prog_->start(), p))  // Match must be leftmost; done.
       return true;
+    // Avoid invoking undefined behavior (arithmetic on a null pointer)
+    // by simply not continuing the loop.
+    if (p == NULL)
+      break;
   }
   return false;
 }
diff --git a/re2/nfa.cc b/re2/nfa.cc
index 3889f118c..77fb5fb18 100644
--- a/re2/nfa.cc
+++ b/re2/nfa.cc
@@ -382,10 +382,15 @@ int NFA::Step(Threadq* runq, Threadq* nextq, int c, const StringPiece& context,
         break;
 
       case kInstMatch: {
-        // Avoid invoking undefined behavior when p happens
-        // to be null - and p-1 would be meaningless anyway.
-        if (p == NULL)
+        // Avoid invoking undefined behavior (arithmetic on a null pointer)
+        // by storing p instead of p-1. (What would the latter even mean?!)
+        // This complements the special case in NFA::Search().
+        if (p == NULL) {
+          CopyCapture(match_, t->capture);
+          match_[1] = p;
+          matched_ = true;
           break;
+        }
 
         if (endmatch_ && p-1 != etext_)
           break;
@@ -593,6 +598,18 @@ bool NFA::Search(const StringPiece& text, const StringPiece& const_context,
         fprintf(stderr, "dead\n");
       break;
     }
+
+    // Avoid invoking undefined behavior (arithmetic on a null pointer)
+    // by simply not continuing the loop.
+    // This complements the special case in NFA::Step().
+    if (p == NULL) {
+      (void)Step(runq, nextq, p < etext_ ? p[0] & 0xFF : -1, context, p);
+      DCHECK_EQ(runq->size(), 0);
+      using std::swap;
+      swap(nextq, runq);
+      nextq->clear();
+      break;
+    }
   }
 
   for (Threadq::iterator i = runq->begin(); i != runq->end(); ++i)
diff --git a/re2/testing/backtrack.cc b/re2/testing/backtrack.cc
index 6cde42d23..1e888da9b 100644
--- a/re2/testing/backtrack.cc
+++ b/re2/testing/backtrack.cc
@@ -148,6 +148,10 @@ bool Backtracker::Search(const StringPiece& text, const StringPiece& context,
     cap_[0] = p;
     if (Visit(prog_->start(), p))  // Match must be leftmost; done.
       return true;
+    // Avoid invoking undefined behavior (arithmetic on a null pointer)
+    // by simply not continuing the loop.
+    if (p == NULL)
+      break;
   }
   return false;
 }