commit bb8e777557ddbdeabdedea4f23613c5021ffd7b1
author Paul Wankadia <junyer@google.com> Mon Nov 25 03:21:08 2019 -0800
committer Paul Wankadia <junyer@google.com> Mon Nov 25 11:22:49 2019 +0000
tree f7ab832818b226eb03bcd9752a834ed9ee23f8c7
parent 4f6e1308c39ca538d4401fac306d72089aa75dfe

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

diff --git a/re2/bitstate.cc b/re2/bitstate.cc
index 5cbc070..f15c1e4 100644
--- a/re2/bitstate.cc
+++ b/re2/bitstate.cc
@@ -342,6 +342,10 @@
     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;
 }

re2/nfa.cc

diff --git a/re2/nfa.cc b/re2/nfa.cc
index 3889f11..77fb5fb 100644
--- a/re2/nfa.cc
+++ b/re2/nfa.cc
@@ -382,10 +382,15 @@
         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 @@
         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)

re2/testing/backtrack.cc

diff --git a/re2/testing/backtrack.cc b/re2/testing/backtrack.cc
index 6cde42d..1e888da 100644
--- a/re2/testing/backtrack.cc
+++ b/re2/testing/backtrack.cc
@@ -148,6 +148,10 @@
     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;
 }