Create out of bounds check for first element of iterator, allowing normal iterator behavior, and fixes filecheck

Author: Jlalond

llvm/include/llvm/Object/Minidump.h

@@ -140,11 +140,8 @@ class MinidumpFile : public Binary {
     size_t Stride;
   };
 
-
-  /// Class the provides an iterator over the memory64 memory ranges. Ranges
-  /// are not validated before hand, and so any increment operation could fail.
-  /// For this reason, the iterator in it's initial state points to a default
-  /// initialized std::pair and should be advanced before dereferencing.
+  /// Class the provides an iterator over the memory64 memory ranges. Only the
+  /// the first descriptor is validated as readable beforehand.
   class Memory64Iterator {
   public:
     static Memory64Iterator
@@ -159,18 +156,18 @@ class MinidumpFile : public Binary {
     std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> Current;
 
     bool operator==(const Memory64Iterator &R) const {
-      return isEnd == R.isEnd;
+      return IsEnd == R.IsEnd;
     }
 
     bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
 
     const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> &
-    operator*() const {
+    operator*() {
       return Current;
     }
 
     const std::pair<minidump::MemoryDescriptor_64, ArrayRef<uint8_t>> *
-    operator->() const {
+    operator&() {
       return &Current;
     }
 
@@ -190,29 +187,37 @@ class MinidumpFile : public Binary {
 
       ArrayRef<uint8_t> Content = Storage.slice(RVA, Descriptor.DataSize);
       Current = std::make_pair(Descriptor, Content);
-      RVA += Descriptor.DataSize;
       Index++;
+      RVA += Descriptor.DataSize;
       if (Index >= Descriptors.size())
-        isEnd = true;
+        IsEnd = true;
       return Error::success();
     }
 
   private:
+    // This constructor will only happen after a validation check to see
+    // if the first descriptor is readable.
     Memory64Iterator(ArrayRef<uint8_t> Storage,
                      ArrayRef<minidump::MemoryDescriptor_64> Descriptors,
                      uint64_t BaseRVA)
         : RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
-          isEnd(false) {}
+          IsEnd(false) {
+      assert(Descriptors.size() > 0);
+      assert(Storage.size() >= BaseRVA + Descriptors.front().DataSize);
+      Current =
+          std::make_pair(Descriptors.front(),
+                         Storage.slice(BaseRVA, Descriptors.front().DataSize));
+    }
 
     Memory64Iterator()
         : RVA(0), Storage(ArrayRef<uint8_t>()),
-          Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), isEnd(true) {}
+          Descriptors(ArrayRef<minidump::MemoryDescriptor_64>()), IsEnd(true) {}
 
     size_t Index = 0;
     uint64_t RVA;
     ArrayRef<uint8_t> Storage;
     ArrayRef<minidump::MemoryDescriptor_64> Descriptors;
-    bool isEnd;
+    bool IsEnd;
   };
 
   using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;

llvm/lib/Object/Minidump.cpp

@@ -175,6 +175,10 @@ MinidumpFile::getMemory64List(Error &Err) const {
   if (!Descriptors)
     return Descriptors.takeError();
 
+  if (!Descriptors->empty() &&
+      ListHeader->BaseRVA + Descriptors->front().DataSize > getData().size())
+    return createEOFError();
+
   return make_range(
       FallibleMemory64Iterator::itr(
           Memory64Iterator::begin(getData(), *Descriptors, ListHeader->BaseRVA),

llvm/lib/ObjectYAML/MinidumpYAML.cpp

@@ -539,13 +539,12 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
     if (!ExpectedList)
       return ExpectedList.takeError();
     std::vector<Memory64ListStream::entry_type> Ranges;
-    for (auto It = ExpectedList->begin(); It != ExpectedList->end();) {
-      // Explicilty advance the iterator before dereference.
-      ++It;
+    for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
       const auto Pair = *It;
-      if (!Err) {
+      if (!Err)
         Ranges.push_back({Pair.first, Pair.second});
-      }
+      else
+        break;
     }
 
     // If we don't have an error, or if any of the reads succeed, return ranges