sql: Fix divide-by-zero in PayloadReader

RecordReader always assumed PayloadReader was initialized properly so I
thought that may have been the issue, but it turns out that the
PayloadReader _was_ being initialized, but max_overflow_payload_size_
was not being set in some cases where it should be. I don't fully
understand this code or if the test case is valid, but failing the read
by returning early should at least be safe (and makes the fuzzer happy)

The extra initialization checks ended up not being needed, but it's
a nice change so I've kept it in the CL

Bug: 1417151
Change-Id: I16bbe4dafc5ddc4c688ca59922c176be36025cc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4264791
Commit-Queue: Austin Sullivan <asully@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1106988}

sql/recover_module/payload.cc

@@ -203,9 +203,16 @@ bool LeafPayloadReader::Initialize(int64_t payload_size, int payload_offset) {
   return true;
 }
 
+bool LeafPayloadReader::IsInitialized() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return page_id_ != DatabasePageReader::kInvalidPageId;
+}
+
 bool LeafPayloadReader::ReadPayload(int64_t offset,
                                     int64_t size,
                                     uint8_t* buffer) {
+  DCHECK(IsInitialized())
+      << "Initialize() not called, or last call did not succeed";
   DCHECK_GE(offset, 0);
   DCHECK_LT(offset, payload_size_);
   DCHECK_GT(size, 0);
@@ -249,6 +256,11 @@ bool LeafPayloadReader::ReadPayload(int64_t offset,
 
   // The read is entirely in overflow pages.
   DCHECK_GE(offset, inline_payload_size_);
+  if (max_overflow_payload_size_ <= 0) {
+    // `max_overflow_payload_size_` should have been set in Initialize() if it's
+    // to be used here. See https://crbug.com/1417151.
+    return false;
+  }
   while (size > 0) {
     const int overflow_page_index =
         (offset - inline_payload_size_) / max_overflow_payload_size_;
@@ -286,7 +298,7 @@ bool LeafPayloadReader::ReadPayload(int64_t offset,
 
 const uint8_t* LeafPayloadReader::ReadInlinePayload() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(page_id_ != DatabasePageReader::kInvalidPageId)
+  DCHECK(IsInitialized())
       << "Initialize() not called, or last call did not succeed";
 
   if (db_reader_->ReadPage(page_id_) != SQLITE_OK)

sql/recover_module/payload.h

@@ -51,6 +51,9 @@ class LeafPayloadReader {
   // class can be called.
   bool Initialize(int64_t payload_size, int payload_offset);
 
+  // True if the last call to Initialize succeeded.
+  bool IsInitialized() const;
+
   // The number of payload bytes that are stored on the B-tree page.
   //
   // The return value is guaranteed to be non-negative and at most
@@ -109,29 +112,29 @@ class LeafPayloadReader {
   const raw_ptr<DatabasePageReader> db_reader_;
 
   // Total size of the current payload.
-  int64_t payload_size_;
+  int64_t payload_size_ = 0;
 
   // The ID of the B-tree page containing the current payload's inline bytes.
   //
   // Set to kInvalidPageId if the reader wasn't successfully initialized.
-  int page_id_;
+  int page_id_ = DatabasePageReader::kInvalidPageId;
 
   // The start of the current payload's inline bytes on the B-tree page.
   //
   // Large payloads extend past the B-tree page containing the payload, via
   // overflow pages.
-  int inline_payload_offset_;
+  int inline_payload_offset_ = 0;
 
   // Number of bytes in the current payload stored in its B-tree page.
   //
   // The rest of the payload is stored on overflow pages.
-  int inline_payload_size_;
+  int inline_payload_size_ = 0;
 
   // Number of overflow pages used by the payload.
-  int overflow_page_count_;
+  int overflow_page_count_ = 0;
 
   // Number of bytes in each overflow page that stores the payload.
-  int max_overflow_payload_size_;
+  int max_overflow_payload_size_ = 0;
 
   // Page IDs for all the payload's overflow pages, in order.
   //

sql/recover_module/record.cc

@@ -154,6 +154,12 @@ bool RecordReader::Initialize() {
   return true;
 }
 
+bool RecordReader::IsInitialized() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return value_headers_.size() == static_cast<size_t>(column_count_) &&
+         payload_reader_->IsInitialized();
+}
+
 ValueType RecordReader::GetValueType(int column_index) const {
   DCHECK(IsInitialized());
   DCHECK_GE(column_index, 0);

sql/recover_module/record.h

@@ -90,10 +90,7 @@ class RecordReader {
   bool Initialize();
 
   // True if the last call to Initialize succeeded.
-  bool IsInitialized() const {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-    return value_headers_.size() == static_cast<size_t>(column_count_);
-  }
+  bool IsInitialized() const;
 
   // The type of a value in the record. |column_index| is 0-based.
   ValueType GetValueType(int column_index) const;