From 1edbde05917d6d7b567b25a7e72142b59546a538 Mon Sep 17 00:00:00 2001 From: SaiGanesh <47469120+ganiganesh25@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:17:49 +0400 Subject: [PATCH] Fix heap-buffer-overflow read in binary PLY list property parsing PlyReader::ParseElementData() read an attacker-controlled list count from a "property list" field and used it to compute the number of bytes to copy out of the input DecoderBuffer, without checking that the buffer had that many bytes remaining. The copy went through buffer->data_head() directly rather than a bounds-checked Decode()/Peek() call, so a crafted binary PLY with a large list count followed by truncated data causes an out-of-bounds heap read. Reachable via the public draco::ReadPointCloudFromFile / ReadMeshFromFile API on untrusted binary_little_endian PLY input. Add bounds checks before the list and non-list property copies, and check the return value of the list-count Decode() call. Add a regression test with a truncated list property. --- src/draco/io/ply_reader.cc | 11 ++++++++++- src/draco/io/ply_reader_test.cc | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) 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