Skip to content

Fix buffer overrun if header specifies more vertices than the file really has #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RamonArguelles wants to merge 1 commit into
base: 2.4
Choose a base branch
from
Open

Fix buffer overrun if header specifies more vertices than the file really has #54

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 27 additions & 14 deletions source/tinyply.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ struct PlyFile::PlyFileImpl
const std::vector<std::string> propertyKeys,
const Type type, const size_t count, uint8_t * data, const Type listType, const size_t listCount);

size_t read_property_binary(const size_t & stride, void * dest, size_t & destOffset, std::istream & is) noexcept;
size_t read_property_ascii(const Type & t, const size_t & stride, void * dest, size_t & destOffset, std::istream & is);
size_t read_property_binary(const size_t & stride, void * dest, size_t & destOffset, size_t destSize, std::istream & is);
size_t read_property_ascii(const Type & t, const size_t & stride, void * dest, size_t & destOffset, size_t destSize, std::istream & is);

std::vector<std::vector<PlyFile::PlyFileImpl::PropertyLookup>> make_property_lookup_table();

Expand Down Expand Up @@ -471,15 +471,25 @@ bool PlyFile::PlyFileImpl::read_header_property(std::istream & is)
return true;
}

size_t PlyFile::PlyFileImpl::read_property_binary(const size_t & stride, void * dest, size_t & destOffset, std::istream & is) noexcept
size_t PlyFile::PlyFileImpl::read_property_binary(const size_t & stride, void * dest, size_t & destOffset, size_t destSize, std::istream & is)
{
if (destOffset + stride > destSize)
{
throw std::runtime_error("unexpected EOF. malformed file?");
}

destOffset += stride;
is.read((char*)dest, stride);
return stride;
}

size_t PlyFile::PlyFileImpl::read_property_ascii(const Type & t, const size_t & stride, void * dest, size_t & destOffset, std::istream & is)
size_t PlyFile::PlyFileImpl::read_property_ascii(const Type & t, const size_t & stride, void * dest, size_t & destOffset, size_t destSize, std::istream & is)
{
if (destOffset + stride > destSize)
{
throw std::runtime_error("unexpected EOF. malformed file?");
}

destOffset += stride;
switch (t)
{
Expand Down Expand Up @@ -858,7 +868,7 @@ void PlyFile::PlyFileImpl::add_properties_to_element(const std::string & element

void PlyFile::PlyFileImpl::parse_data(std::istream & is, bool firstPass)
{
std::function<size_t(PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, std::istream & is)> read;
std::function<size_t(PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, size_t destSize, std::istream & is)> read;
std::function<size_t(PropertyLookup & f, const PlyProperty & p, std::istream & is)> skip;

const auto start = is.tellg();
Expand Down Expand Up @@ -894,11 +904,11 @@ void PlyFile::PlyFileImpl::parse_data(std::istream & is, bool firstPass)

if (isBinary)
{
read = [this, &listSize, &dummyCount, &read_list_binary](PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, std::istream & _is) noexcept
read = [this, &listSize, &dummyCount, &read_list_binary](PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, size_t destSize, std::istream & _is)
{
if (!p.isList) return read_property_binary(f.prop_stride, dest + destOffset, destOffset, _is);
if (!p.isList) return read_property_binary(f.prop_stride, dest + destOffset, destOffset, destSize, _is);
read_list_binary(p.listType, &listSize, dummyCount, f.list_stride, _is); // the list size
return read_property_binary(f.prop_stride * listSize, dest + destOffset, destOffset, _is); // properties in list
return read_property_binary(f.prop_stride * listSize, dest + destOffset, destOffset, destSize, _is); // properties in list
};
skip = [&listSize, &dummyCount, &read_list_binary](PropertyLookup & f, const PlyProperty & p, std::istream & _is) noexcept
{
Expand All @@ -915,19 +925,20 @@ void PlyFile::PlyFileImpl::parse_data(std::istream & is, bool firstPass)
}
else
{
read = [this, &listSize, &dummyCount](PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, std::istream & _is) noexcept
read = [this, &listSize, &dummyCount](PropertyLookup & f, const PlyProperty & p, uint8_t * dest, size_t & destOffset, size_t destSize, std::istream & _is)
{
if (!p.isList)
{
return read_property_ascii(p.propertyType, f.prop_stride, dest + destOffset, destOffset, _is);
return read_property_ascii(p.propertyType, f.prop_stride, dest + destOffset, destOffset, destSize, _is);
}
else
{
read_property_ascii(p.listType, f.list_stride, &listSize, dummyCount, _is); // the list size
dummyCount = 0;
read_property_ascii(p.listType, f.list_stride, &listSize, dummyCount, sizeof(listSize), _is); // the list size
size_t bytes_read = 0;
for (size_t i = 0; i < listSize; ++i)
{
bytes_read += read_property_ascii(p.propertyType, f.prop_stride, dest + destOffset, destOffset, _is);
bytes_read += read_property_ascii(p.propertyType, f.prop_stride, dest + destOffset, destOffset, destSize, _is);
}
return bytes_read;
}
Expand All @@ -937,7 +948,8 @@ void PlyFile::PlyFileImpl::parse_data(std::istream & is, bool firstPass)
skip_ascii_buffer.clear();
if (p.isList)
{
read_property_ascii(p.listType, f.list_stride, &listSize, dummyCount, _is); // the list size (does not count for memory alloc)
dummyCount = 0;
read_property_ascii(p.listType, f.list_stride, &listSize, dummyCount, sizeof(listSize), _is); // the list size (does not count for memory alloc)
for (size_t i = 0; i < listSize; ++i) _is >> skip_ascii_buffer; // properties in list
return listSize * f.prop_stride;
}
Expand Down Expand Up @@ -973,7 +985,8 @@ void PlyFile::PlyFileImpl::parse_data(std::istream & is, bool firstPass)
}
else
{
const size_t num_bytes = read(lookup, property, helper->data->buffer.get(), helper->cursor->byteOffset, is);
const size_t destSize = helper->data->buffer.size_bytes();
const size_t num_bytes = read(lookup, property, helper->data->buffer.get(), helper->cursor->byteOffset, destSize, is);
total_bytes_parsed += num_bytes;
bytes_since_callback += num_bytes;
}
Expand Down