From 47e3bf01bab142e3fc83aadcfa39d79a00a4aeca Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 27 Dec 2017 16:53:03 -0800 Subject: [PATCH] indexcodec: Make meshopt_encodeIndexBuffer memory-safe The encoder now does all appropriate bounds checks to make sure that we either can encode the data safely within the provided space, or return 0 if there isn't sufficient space. This makes it possible to encode data using more constrained buffers if necessary. Note that right now providing the exact buffer size is enough to encode any input since the encoder is single-pass; if we switch to a multi-pass encoder this invariant might get violated depending on the implementation. --- src/indexcodec.cpp | 16 +++++++++++++++- src/meshoptimizer.h | 4 +++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/indexcodec.cpp b/src/indexcodec.cpp index 47559710..8b650b4e 100644 --- a/src/indexcodec.cpp +++ b/src/indexcodec.cpp @@ -147,6 +147,10 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons assert(index_count % 3 == 0); + // the minimum valid encoding is 1 byte per triangle and a 16-byte codeaux table + if (buffer_size < index_count / 3 + 16) + return 0; + EdgeFifo edgefifo; memset(edgefifo, -1, sizeof(edgefifo)); @@ -161,6 +165,7 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons unsigned char* code = buffer; unsigned char* data = buffer + index_count / 3; + unsigned char* data_safe_end = buffer + buffer_size - 16; // use static encoding table; it's possible to pack the result and then build an optimal table and repack // for now we keep it simple and use the table that has been generated based on symbol frequency on a training mesh set @@ -172,6 +177,12 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons for (size_t i = 0; i < index_count; i += 3) { + // make sure we have enough space to write a triangle + // each triangle writes at most 16 bytes: 1b for codeaux and 5b for each free index + // after this we can be sure we can write without extra bounds checks + if (data > data_safe_end) + return 0; + int fer = getEdgeFifo(edgefifo, indices[i + 0], indices[i + 1], indices[i + 2], edgefifooffset); if (fer >= 0 && (fer >> 2) < 15) @@ -256,6 +267,10 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons } } + // make sure we have enough space to write codeaux table + if (data > data_safe_end) + return 0; + // add codeaux encoding table to the end of the stream; this is used for decoding codeaux *and* as padding // we need padding for decoding to be able to assume that each triangle is encoded as <= 16 bytes of extra data // this is enough space for aux byte + 5 bytes per varint index which is the absolute worst case for any input @@ -266,7 +281,6 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons assert(data >= buffer + index_count / 3 + 16); assert(data <= buffer + buffer_size); - (void)buffer_size; return data - buffer; } diff --git a/src/meshoptimizer.h b/src/meshoptimizer.h index ffd3fedb..b0e2e72c 100644 --- a/src/meshoptimizer.h +++ b/src/meshoptimizer.h @@ -88,6 +88,7 @@ MESHOPTIMIZER_API size_t meshopt_optimizeVertexFetch(void* destination, unsigned /** * Experimental: Index buffer encoder * Encodes index data into an array of bytes that is generally much smaller (<1.5 bytes/triangle) and compresses better (<1 bytes/triangle) compared to original. + * Returns encoded data size on success, 0 on error; the only error condition is if buffer doesn't have enough space * For maximum efficiency the index buffer being encoded has to be optimized for vertex cache and vertex fetch first. * * buffer must contain enough space for the encoded index buffer (use meshopt_encodeIndexBufferBound to estimate) @@ -98,7 +99,7 @@ MESHOPTIMIZER_API size_t meshopt_encodeIndexBufferBound(size_t index_count, size /** * Experimental: Index buffer decoder * Decodes index data from an array of bytes generated by meshopt_encodeIndexBuffer - * Returns 0 if encoding is successful, and an error code otherwise + * Returns 0 if decoding was successful, and an error code otherwise * * destination must contain enough space for the resulting index buffer (index_count elements) */ @@ -118,6 +119,7 @@ MESHOPTIMIZER_API size_t meshopt_simplify(unsigned int* destination, const unsig * Experimental: Mesh stripifier * Converts a previously vertex cache optimized triangle list to triangle strip, stitching strips using restart index * Returns the number of indices in the resulting strip, with destination containing new index data + * For maximum efficiency the index buffer being converted has to be optimized for vertex cache first. * * destination must contain enough space for the worst case target index buffer (index_count / 3 * 4 elements) */