diff --git a/libs/filameshio/include/filameshio/MeshReader.h b/libs/filameshio/include/filameshio/MeshReader.h index 2253379120d7..cc195be943d3 100644 --- a/libs/filameshio/include/filameshio/MeshReader.h +++ b/libs/filameshio/include/filameshio/MeshReader.h @@ -100,19 +100,43 @@ class UTILS_PUBLIC MeshReader { * file cannot be matched to a material in the registry, a default material is * used instead. The default material can be overridden by adding a material * named "DefaultMaterial" to the registry. + * + * This overload does NOT validate reads against the buffer size. Prefer the + * overload that accepts dataSize when loading data from an untrusted source. */ static Mesh loadMeshFromBuffer(filament::Engine* engine, void const* data, Callback destructor, void* user, MaterialRegistry& materials); + /** + * Loads a filamesh renderable from an in-memory buffer, validating that all + * reads stay within dataSize bytes of data. Returns an empty Mesh on any + * malformed header field or truncated payload. + */ + static Mesh loadMeshFromBuffer(filament::Engine* engine, + void const* data, size_t dataSize, Callback destructor, void* user, + MaterialRegistry& materials); + /** * Loads a filamesh renderable from an in-memory buffer. The material registry * can be used to provide named materials. All the primitives of the decoded * renderable are assigned the specified default material. + * + * This overload does NOT validate reads against the buffer size. Prefer the + * overload that accepts dataSize when loading data from an untrusted source. */ static Mesh loadMeshFromBuffer(filament::Engine* engine, void const* data, Callback destructor, void* user, filament::MaterialInstance* defaultMaterial); + + /** + * Loads a filamesh renderable from an in-memory buffer, validating that all + * reads stay within dataSize bytes of data. All the primitives of the decoded + * renderable are assigned the specified default material. + */ + static Mesh loadMeshFromBuffer(filament::Engine* engine, + void const* data, size_t dataSize, Callback destructor, void* user, + filament::MaterialInstance* defaultMaterial); }; } // namespace filamesh diff --git a/libs/filameshio/src/MeshReader.cpp b/libs/filameshio/src/MeshReader.cpp index 700856177db0..8f97959f59af 100644 --- a/libs/filameshio/src/MeshReader.cpp +++ b/libs/filameshio/src/MeshReader.cpp @@ -32,10 +32,11 @@ #include #include -#include -#include +#include +#include #include #include +#include #include #if !defined(WIN32) @@ -78,7 +79,7 @@ MeshReader::MaterialRegistry::~MaterialRegistry() { } // Default move construction -MeshReader::MaterialRegistry::MaterialRegistry(MaterialRegistry&& rhs) +MeshReader::MaterialRegistry::MaterialRegistry(MaterialRegistry&& rhs) : mImpl(nullptr) { std::swap(mImpl, rhs.mImpl); } @@ -165,6 +166,10 @@ MeshReader::Mesh MeshReader::loadMeshFromFile(filament::Engine* engine, const ut Mesh mesh; int fd = open(path.c_str(), O_RDONLY); + if (fd < 0) { + utils::slog.e << "Unable to open mesh file." << utils::io::endl; + return mesh; + } size_t size = fileSize(fd); char* data = (char*) malloc(size); @@ -176,7 +181,7 @@ MeshReader::Mesh MeshReader::loadMeshFromFile(filament::Engine* engine, const ut p += sizeof(MAGICID); if (!strncmp(MAGICID, magic, 8)) { - mesh = loadMeshFromBuffer(engine, data, nullptr, nullptr, materials); + mesh = loadMeshFromBuffer(engine, data, size, nullptr, nullptr, materials); } Fence::waitAndDestroy(engine->createFence()); @@ -187,68 +192,165 @@ MeshReader::Mesh MeshReader::loadMeshFromFile(filament::Engine* engine, const ut return mesh; } +// Backward-compatible overload: no buffer-size information is provided, so no +// buffer-end bounds checking is performed. Callers with untrusted data should +// use the overload that accepts dataSize. +MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, + void const* data, Callback destructor, void* user, + MaterialInstance* defaultMaterial) { + return loadMeshFromBuffer(engine, data, SIZE_MAX, destructor, user, defaultMaterial); +} + MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, void const* data, Callback destructor, void* user, + MaterialRegistry& materials) { + return loadMeshFromBuffer(engine, data, SIZE_MAX, destructor, user, materials); +} + +MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, + void const* data, size_t dataSize, Callback destructor, void* user, MaterialInstance* defaultMaterial) { MaterialRegistry reg; reg.registerMaterialInstance(utils::CString(DEFAULT_MATERIAL), defaultMaterial); - return loadMeshFromBuffer(engine, data, destructor, user, reg); + return loadMeshFromBuffer(engine, data, dataSize, destructor, user, reg); } MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, - void const* data, Callback destructor, void* user, + void const* data, size_t dataSize, Callback destructor, void* user, MaterialRegistry& materials) { - const uint8_t* p = (const uint8_t*) data; - if (strncmp(MAGICID, (const char *) p, 8)) { + const uint8_t* const base = (const uint8_t*) data; + size_t consumed = 0; + + // Magic. + if (dataSize < sizeof(MAGICID) || strncmp(MAGICID, (const char*) base, sizeof(MAGICID))) { utils::slog.e << "Magic string not found." << utils::io::endl; return {}; } - p += 8; + consumed = sizeof(MAGICID); + + // Header: copy into a local to avoid strict-aliasing and alignment UB when + // the caller-supplied buffer is not naturally aligned for uint32_t fields. + if (dataSize - consumed < sizeof(Header)) { + utils::slog.e << "Buffer too small for header." << utils::io::endl; + return {}; + } + Header header; + memcpy(&header, base + consumed, sizeof(Header)); + consumed += sizeof(Header); - Header* header = (Header*) p; - p += sizeof(Header); + if (header.version != VERSION) { + utils::slog.e << "Unsupported filamesh version: " << header.version << utils::io::endl; + return {}; + } - uint8_t const* vertexData = p; - p += header->vertexSize; + // Vertex data. + if (header.vertexSize > dataSize - consumed) { + utils::slog.e << "Invalid vertexSize." << utils::io::endl; + return {}; + } + const uint8_t* vertexData = base + consumed; + consumed += header.vertexSize; - uint8_t const* indices = p; - p += header->indexSize; + // Index data. + if (header.indexSize > dataSize - consumed) { + utils::slog.e << "Invalid indexSize." << utils::io::endl; + return {}; + } + const uint8_t* indices = base + consumed; + consumed += header.indexSize; - Part* parts = (Part*) p; - p += header->parts * sizeof(Part); + // Parts. + if (header.parts > SIZE_MAX / sizeof(Part)) { + utils::slog.e << "Mesh part count overflows." << utils::io::endl; + return {}; + } + const size_t partsBytes = size_t(header.parts) * sizeof(Part); + if (partsBytes > dataSize - consumed) { + utils::slog.e << "Invalid parts count." << utils::io::endl; + return {}; + } + const uint8_t* const partsBase = base + consumed; + consumed += partsBytes; - uint32_t materialCount = (uint32_t) *p; - p += sizeof(uint32_t); + // Material count. The writer emits a 4-byte uint32_t; read it as such + // instead of dereferencing a single byte. + if (dataSize - consumed < sizeof(uint32_t)) { + utils::slog.e << "Buffer too small for material count." << utils::io::endl; + return {}; + } + uint32_t materialCount; + memcpy(&materialCount, base + consumed, sizeof(uint32_t)); + consumed += sizeof(uint32_t); std::vector partsMaterial(materialCount); for (size_t i = 0; i < materialCount; i++) { - uint32_t nameLength = (uint32_t) *p; - p += sizeof(uint32_t); - partsMaterial[i] = (const char*) p; - p += nameLength + 1; // null terminated + if (dataSize - consumed < sizeof(uint32_t)) { + utils::slog.e << "Buffer too small for material name length." << utils::io::endl; + return {}; + } + uint32_t nameLength; + memcpy(&nameLength, base + consumed, sizeof(uint32_t)); + consumed += sizeof(uint32_t); + + // Bound nameLength against the remaining buffer before adding 1 so + // the add cannot wrap on 32-bit targets where size_t is 32-bit. + if (nameLength >= dataSize - consumed) { + utils::slog.e << "Invalid material name length." << utils::io::endl; + return {}; + } + // Copy exactly nameLength bytes. Assigning from a bare C-string + // pointer would fall back to strlen and could read past the declared + // span if the buffer is not NUL-terminated at the expected offset. + partsMaterial[i].assign((const char*) (base + consumed), nameLength); + consumed += size_t(nameLength) + 1; } Mesh mesh; + // Pre-compute compressed-buffer sizing so we can reject malformed headers + // before allocating any engine-owned resources (which would otherwise + // leak on early return). + constexpr uint32_t uintmax = std::numeric_limits::max(); + const bool hasUV1 = header.offsetUV1 != uintmax && header.strideUV1 != uintmax; + const size_t indexStride = header.indexType == UI16 ? sizeof(uint16_t) : sizeof(uint32_t); + const size_t vertexStride = sizeof(half4) + sizeof(short4) + sizeof(ubyte4) + sizeof(ushort2) + + (hasUV1 ? sizeof(ushort2) : 0); + if (header.flags & COMPRESSION) { + if (header.indexCount > 0 && indexStride > SIZE_MAX / header.indexCount) { + utils::slog.e << "Index buffer size overflow." << utils::io::endl; + return {}; + } + if (header.vertexCount > 0 && vertexStride > SIZE_MAX / header.vertexCount) { + utils::slog.e << "Vertex buffer size overflow." << utils::io::endl; + return {}; + } + } + mesh.indexBuffer = IndexBuffer::Builder() - .indexCount(header->indexCount) - .bufferType(header->indexType == UI16 ? IndexBuffer::IndexType::USHORT + .indexCount(header.indexCount) + .bufferType(header.indexType == UI16 ? IndexBuffer::IndexType::USHORT : IndexBuffer::IndexType::UINT) .build(*engine); // If the index buffer is compressed, then decode the indices into a temporary buffer. // The user callback can be called immediately afterwards because the source data does not get // passed to the GPU. - const size_t indicesSize = header->indexSize; - if (header->flags & COMPRESSION) { - size_t indexSize = header->indexType == UI16 ? sizeof(uint16_t) : sizeof(uint32_t); - size_t indexCount = header->indexCount; - size_t uncompressedSize = indexSize * indexCount; + const size_t indicesSize = header.indexSize; + if (header.flags & COMPRESSION) { + const size_t indexCount = header.indexCount; + const size_t uncompressedSize = indexStride * indexCount; void* uncompressed = malloc(uncompressedSize); - int err = meshopt_decodeIndexBuffer(uncompressed, indexCount, indexSize, indices, + if (!uncompressed) { + utils::slog.e << "Failed to allocate index buffer." << utils::io::endl; + engine->destroy(mesh.indexBuffer); + return {}; + } + int err = meshopt_decodeIndexBuffer(uncompressed, indexCount, indexStride, indices, indicesSize); if (err) { + free(uncompressed); utils::slog.e << "Unable to decode index buffer." << utils::io::endl; + engine->destroy(mesh.indexBuffer); return {}; } if (destructor) { @@ -263,32 +365,29 @@ MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, } VertexBuffer::Builder vbb; - vbb.vertexCount(header->vertexCount) + vbb.vertexCount(header.vertexCount) .bufferCount(1) .normalized(VertexAttribute::COLOR) .normalized(VertexAttribute::TANGENTS); - VertexBuffer::AttributeType uvtype = (header->flags & TEXCOORD_SNORM16) ? + VertexBuffer::AttributeType uvtype = (header.flags & TEXCOORD_SNORM16) ? VertexBuffer::AttributeType::SHORT2 : VertexBuffer::AttributeType::HALF2; vbb .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::HALF4, - header->offsetPosition, uint8_t(header->stridePosition)) + header.offsetPosition, uint8_t(header.stridePosition)) .attribute(VertexAttribute::TANGENTS, 0, VertexBuffer::AttributeType::SHORT4, - header->offsetTangents, uint8_t(header->strideTangents)) + header.offsetTangents, uint8_t(header.strideTangents)) .attribute(VertexAttribute::COLOR, 0, VertexBuffer::AttributeType::UBYTE4, - header->offsetColor, uint8_t(header->strideColor)) + header.offsetColor, uint8_t(header.strideColor)) .attribute(VertexAttribute::UV0, 0, uvtype, - header->offsetUV0, uint8_t(header->strideUV0)) - .normalized(VertexAttribute::UV0, header->flags & TEXCOORD_SNORM16); - - constexpr uint32_t uintmax = std::numeric_limits::max(); - const bool hasUV1 = header->offsetUV1 != uintmax && header->strideUV1 != uintmax; + header.offsetUV0, uint8_t(header.strideUV0)) + .normalized(VertexAttribute::UV0, header.flags & TEXCOORD_SNORM16); if (hasUV1) { vbb .attribute(VertexAttribute::UV1, 0, VertexBuffer::AttributeType::HALF2, - header->offsetUV1, uint8_t(header->strideUV1)) + header.offsetUV1, uint8_t(header.strideUV1)) .normalized(VertexAttribute::UV1); } @@ -297,45 +396,53 @@ MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, // If the vertex buffer is compressed, then decode the vertices into a temporary buffer. // The user callback can be called immediately afterwards because the source data does not get // passed to the GPU. - const size_t verticesSize = header->vertexSize; - if (header->flags & COMPRESSION) { - size_t vertexSize = sizeof(half4) + sizeof(short4) + sizeof(ubyte4) + sizeof(ushort2) + - (hasUV1 ? sizeof(ushort2) : 0); - size_t vertexCount = header->vertexCount; - size_t uncompressedSize = vertexSize * vertexCount; + const size_t verticesSize = header.vertexSize; + if (header.flags & COMPRESSION) { + const size_t vertexCount = header.vertexCount; + const size_t uncompressedSize = vertexStride * vertexCount; void* uncompressed = malloc(uncompressedSize); + if (!uncompressed) { + utils::slog.e << "Failed to allocate vertex buffer." << utils::io::endl; + engine->destroy(mesh.indexBuffer); + engine->destroy(mesh.vertexBuffer); + return {}; + } const uint8_t* srcdata = vertexData + sizeof(CompressionHeader); int err = 0; - if (header->flags & INTERLEAVED) { - err |= meshopt_decodeVertexBuffer(uncompressed, vertexCount, vertexSize, srcdata, - vertexSize); + if (header.flags & INTERLEAVED) { + err |= meshopt_decodeVertexBuffer(uncompressed, vertexCount, vertexStride, srcdata, + vertexStride); } else { - const CompressionHeader* sizes = (CompressionHeader*) vertexData; + CompressionHeader sizes; + memcpy(&sizes, vertexData, sizeof(CompressionHeader)); uint8_t* dstdata = (uint8_t*) uncompressed; auto decode = meshopt_decodeVertexBuffer; - err |= decode(dstdata, vertexCount, sizeof(half4), srcdata, sizes->positions); - srcdata += sizes->positions; + err |= decode(dstdata, vertexCount, sizeof(half4), srcdata, sizes.positions); + srcdata += sizes.positions; dstdata += sizeof(half4) * vertexCount; - err |= decode(dstdata, vertexCount, sizeof(short4), srcdata, sizes->tangents); - srcdata += sizes->tangents; + err |= decode(dstdata, vertexCount, sizeof(short4), srcdata, sizes.tangents); + srcdata += sizes.tangents; dstdata += sizeof(short4) * vertexCount; - err |= decode(dstdata, vertexCount, sizeof(ubyte4), srcdata, sizes->colors); - srcdata += sizes->colors; + err |= decode(dstdata, vertexCount, sizeof(ubyte4), srcdata, sizes.colors); + srcdata += sizes.colors; dstdata += sizeof(ubyte4) * vertexCount; - err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes->uv0); + err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes.uv0); - if (sizes->uv1) { - srcdata += sizes->uv0; + if (sizes.uv1) { + srcdata += sizes.uv0; dstdata += sizeof(ushort2) * vertexCount; - err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes->uv1); + err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes.uv1); } } if (err) { + free(uncompressed); utils::slog.e << "Unable to decode vertex buffer." << utils::io::endl; + engine->destroy(mesh.indexBuffer); + engine->destroy(mesh.vertexBuffer); return {}; } if (destructor) { @@ -351,18 +458,21 @@ MeshReader::Mesh MeshReader::loadMeshFromBuffer(filament::Engine* engine, mesh.renderable = utils::EntityManager::get().create(); - RenderableManager::Builder builder(header->parts); - builder.boundingBox(header->aabb); + RenderableManager::Builder builder(header.parts); + builder.boundingBox(header.aabb); const auto defaultmi = materials.getMaterialInstance(utils::CString(DEFAULT_MATERIAL)); - for (size_t i = 0; i < header->parts; i++) { + for (size_t i = 0; i < header.parts; i++) { + Part part; + memcpy(&part, partsBase + i * sizeof(Part), sizeof(Part)); + builder.geometry(i, RenderableManager::PrimitiveType::TRIANGLES, - mesh.vertexBuffer, mesh.indexBuffer, parts[i].offset, - parts[i].minIndex, parts[i].maxIndex, parts[i].indexCount); + mesh.vertexBuffer, mesh.indexBuffer, part.offset, + part.minIndex, part.maxIndex, part.indexCount); // It may happen that there are more parts than materials // therefore we have to use Part::material instead of i. - uint32_t materialIndex = parts[i].material; + uint32_t materialIndex = part.material; if (materialIndex >= partsMaterial.size()) { utils::slog.e << "Material index (" << materialIndex << ") of mesh part (" << i << ") is out of bounds (" << partsMaterial.size() << ")" << utils::io::endl; diff --git a/libs/filameshio/tests/test_filamesh.cpp b/libs/filameshio/tests/test_filamesh.cpp index 57d3db26332c..b41319f63495 100644 --- a/libs/filameshio/tests/test_filamesh.cpp +++ b/libs/filameshio/tests/test_filamesh.cpp @@ -157,7 +157,8 @@ TEST_F(FilameshTest, NonInterleaved) { // Deserialize the mesh as a smoke test. MaterialInstance* mi = engine->getDefaultMaterial()->createInstance(); - auto mesh = MeshReader::loadMeshFromBuffer(engine, stream.str().data(), nullptr, nullptr, mi); + auto mesh = MeshReader::loadMeshFromBuffer(engine, stream.str().data(), stream.str().size(), + nullptr, nullptr, mi); auto& rm = engine->getRenderableManager(); auto inst = rm.getInstance(mesh.renderable); EXPECT_EQ(rm.getPrimitiveCount(inst), 1); @@ -206,7 +207,8 @@ TEST_F(FilameshTest, Interleaved) { // Deserialize the mesh as a smoke test. MaterialInstance* mi = engine->getDefaultMaterial()->createInstance(); - auto mesh = MeshReader::loadMeshFromBuffer(engine, stream.str().data(), nullptr, nullptr, mi); + auto mesh = MeshReader::loadMeshFromBuffer(engine, stream.str().data(), stream.str().size(), + nullptr, nullptr, mi); auto& rm = engine->getRenderableManager(); auto inst = rm.getInstance(mesh.renderable); EXPECT_EQ(rm.getPrimitiveCount(inst), 1); diff --git a/libs/image/src/LinearImage.cpp b/libs/image/src/LinearImage.cpp index 112e2002e9a4..7f75cd6e8d26 100644 --- a/libs/image/src/LinearImage.cpp +++ b/libs/image/src/LinearImage.cpp @@ -16,16 +16,28 @@ #include +#include #include // for memset #include +#include namespace image { struct LinearImage::SharedReference { SharedReference(uint32_t width, uint32_t height, uint32_t channels) { - const uint32_t nfloats = width * height * channels; - float* floats = new float[nfloats]; - memset(floats, 0, sizeof(float) * nfloats); + // width * height fits in uint64_t; the subsequent multiplication by + // channels can still overflow uint64_t, so saturate in two stages. + const uint64_t wh = uint64_t(width) * height; + if (channels != 0 && wh > UINT64_MAX / channels) { + throw std::runtime_error("LinearImage dimensions too large"); + } + const uint64_t nfloats = wh * channels; + if (nfloats > SIZE_MAX / sizeof(float)) { + throw std::runtime_error("LinearImage dimensions too large"); + } + const size_t n = static_cast(nfloats); + float* floats = new float[n]; + memset(floats, 0, sizeof(float) * n); pixels = std::shared_ptr(floats, std::default_delete()); } std::shared_ptr pixels;