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 <[email protected]>
google · Nov 25, 2019 · bb8e777 · bb8e777
1 parent 4f6e130
commit bb8e777
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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;
 }