Fix NPE caused by handling GetFileSize return value.android13-dev

FileBackedVector wasn't properly checking the return value of
GetFileSize when deciding whether or not it needed to grow the
underlying file and remap. If GetFileSize failed due to an IO error,
then FileBackedVector would conclude (incorrectly) that growing wasn't
necessary and would attempt to access the mmapping at the requested
index - which could result in either an NPE (if the vector was empty) or
an invalid memory access (if the requested index exceeded the actual
size of the mmapping). See b/232273174#comment44 for more details.

This is a cherrypick of cl/451040413.

Test: Builds
Change-Id: I7ae0e3119126483c23a85dae3bd7a84712357209

2 files changed, 25 insertions, 1 deletions

diff --git a/icing/file/file-backed-vector.h b/icing/file/file-backed-vector.h
index 00bdc7e..7e42e32 100644
--- a/icing/file/file-backed-vector.h
+++ b/icing/file/file-backed-vector.h
@@ -586,8 +586,11 @@ libtextclassifier3::Status FileBackedVector<T>::GrowIfNecessary(
   }
 
   int64_t current_file_size = filesystem_->GetFileSize(file_path_.c_str());
-  int64_t least_file_size_needed = sizeof(Header) + num_elements * sizeof(T);
+  if (current_file_size == Filesystem::kBadFileSize) {
+    return absl_ports::InternalError("Unable to retrieve file size.");
+  }
 
+  int64_t least_file_size_needed = sizeof(Header) + num_elements * sizeof(T);
   if (least_file_size_needed <= current_file_size) {
     // Our underlying file can hold the target num_elements cause we've grown
     // before
diff --git a/icing/file/file-backed-vector_test.cc b/icing/file/file-backed-vector_test.cc
index 54f9ef5..ed94fa5 100644
--- a/icing/file/file-backed-vector_test.cc
+++ b/icing/file/file-backed-vector_test.cc
@@ -677,6 +677,27 @@ TEST_F(FileBackedVectorTest, RemapFailureStillValidInstance) {
   EXPECT_THAT(vector->Get(kResizingIndex / 2), IsOkAndHolds(Pointee(Eq(9))));
 }
 
+TEST_F(FileBackedVectorTest, BadFileSizeDuringGrowReturnsError) {
+  auto mock_filesystem = std::make_unique<MockFilesystem>();
+  ICING_ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<FileBackedVector<int>> vector,
+      FileBackedVector<int>::Create(
+          *mock_filesystem, file_path_,
+          MemoryMappedFile::Strategy::READ_WRITE_AUTO_SYNC));
+
+  // At first, the vector is empty and has no mapping established. The first Set
+  // call will cause a Grow.
+  // During Grow, we will attempt to check the underlying file size to see if
+  // growing is actually necessary. Return an error on the call to GetFileSize.
+  ON_CALL(*mock_filesystem, GetFileSize(A<const char*>()))
+      .WillByDefault(Return(Filesystem::kBadFileSize));
+
+  // We should fail gracefully and return an INTERNAL error to indicate that
+  // there was an issue retrieving the file size.
+  EXPECT_THAT(vector->Set(0, 7),
+              StatusIs(libtextclassifier3::StatusCode::INTERNAL));
+}
+
 }  // namespace
 
 }  // namespace lib