diff --git a/src/draco/io/ply_reader.cc b/src/draco/io/ply_reader.cc index 0da9ab5fe..249db3e8f 100644 --- a/src/draco/io/ply_reader.cc +++ b/src/draco/io/ply_reader.cc @@ -194,7 +194,9 @@ bool PlyReader::ParseElementData(DecoderBuffer *buffer, int element_index) { if (prop.is_list()) { // Parse the number of entries for the list element. int64_t num_entries = 0; - buffer->Decode(&num_entries, prop.list_data_type_num_bytes()); + if (!buffer->Decode(&num_entries, prop.list_data_type_num_bytes())) { + return false; + } // Store offset to the main data entry. prop.list_data_.push_back(prop.data_.size() / prop.data_type_num_bytes_); @@ -203,11 +205,18 @@ bool PlyReader::ParseElementData(DecoderBuffer *buffer, int element_index) { // Read and store the actual property data const int64_t num_bytes_to_read = prop.data_type_num_bytes() * num_entries; + if (num_bytes_to_read < 0 || + num_bytes_to_read > buffer->remaining_size()) { + return false; + } prop.data_.insert(prop.data_.end(), buffer->data_head(), buffer->data_head() + num_bytes_to_read); buffer->Advance(num_bytes_to_read); } else { // Non-list property + if (prop.data_type_num_bytes() > buffer->remaining_size()) { + return false; + } prop.data_.insert(prop.data_.end(), buffer->data_head(), buffer->data_head() + prop.data_type_num_bytes()); buffer->Advance(prop.data_type_num_bytes()); diff --git a/src/draco/io/ply_reader_test.cc b/src/draco/io/ply_reader_test.cc index 9612f6377..5ef24d361 100644 --- a/src/draco/io/ply_reader_test.cc +++ b/src/draco/io/ply_reader_test.cc @@ -140,4 +140,31 @@ TEST_F(PlyReaderTest, TestReaderMoreDataTypes) { } } +TEST_F(PlyReaderTest, TestReaderTruncatedListData) { + // Binary PLY where the "face" element declares a list property whose + // count field claims far more entries than the remaining buffer can hold. + // Regression test for a heap-buffer-overflow read in + // PlyReader::ParseElementData(): the list count was previously used to + // copy data out of the input buffer without a bounds check. + const char kData[] = + "ply\n" + "format binary_little_endian 1.0\n" + "element vertex 1\n" + "property float x\n" + "property float y\n" + "property float z\n" + "element face 1\n" + "property list uchar int vertex_indices\n" + "end_header\n" + "\x00\x00\x80\x3f\x00\x00\x00\x40\x00\x00\x40\x40" // vertex: 1, 2, 3 + "\xff" // list count claims 255 int32 entries (1020 bytes) + "\x41\x41" // but only 2 bytes of data actually follow + ; + DecoderBuffer buf; + buf.Init(kData, sizeof(kData) - 1); + PlyReader reader; + const Status status = reader.Read(&buf); + ASSERT_FALSE(status.ok()); +} + } // namespace draco