indexcodec: Make decoder memory-safe for arbitrary input data

We now do proper bounds checks to make sure that the decoder can be
safely used without memory safety issues. To reach that without
compromising performance, we make sure that there's always 16 bytes of
data to read for any triangle (worst case 1 aux byte + 3*5 varint
bytes). Decoder uses that to do one bounds check per triangle.

In addition, the varbyte decoder had to be changed to guarantee that it
reads at most 5 bytes from the stream.

Note: the decoder hasn't been fuzzed yet so it's possible that there
still are memory safety issues that aren't obvious.

demo/main.cpp

@@ -285,7 +285,9 @@ void encodeIndex(const Mesh& mesh)
 	clock_t middle = clock();
 
 	std::vector<uint32_t> result(mesh.indices.size());
-	meshopt_decodeIndexBuffer(&result[0], mesh.indices.size(), &buffer[0], buffer.size());
+	int res = meshopt_decodeIndexBuffer(&result[0], mesh.indices.size(), &buffer[0], buffer.size());
+	assert(res == 0);
+	(void)res;
 
 	clock_t end = clock();
 

src/indexcodec.cpp

@@ -90,16 +90,23 @@ static void encodeVByte(unsigned char*& data, unsigned int v)
 
 static unsigned int decodeVByte(const unsigned char*& data)
 {
-	unsigned int result = 0;
-	unsigned int shift = 0;
-	unsigned char group;
+	unsigned char lead = *data++;
 
-	do
+	if (lead < 128)
+		return lead;
+
+	unsigned int result = lead & 127;
+	unsigned int shift = 7;
+
+	for (int i = 0; i < 4; ++i)
 	{
-		group = *data++;
+		unsigned char group = *data++;
 		result |= (group & 127) << shift;
 		shift += 7;
-	} while (group & 128);
+
+		if (group < 128)
+			break;
+	}
 
 	return result;
 }
@@ -238,6 +245,14 @@ size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_size, cons
 		}
 	}
 
+	// add 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
+	for (size_t i = 0; i < 16; ++i)
+	{
+		*data++ = 0;
+	}
+
+	assert(data >= buffer + index_count / 3 + 16);
 	assert(data <= buffer + buffer_size);
 	(void)buffer_size;
 
@@ -257,15 +272,18 @@ size_t meshopt_encodeIndexBufferBound(size_t index_count, size_t vertex_count)
 	// worst-case encoding is 2 header bytes + 3 varint-7 encoded index deltas
 	unsigned int vertex_groups = (vertex_bits + 1 + 6) / 7;
 
-	return (index_count / 3) * (2 + 3 * vertex_groups);
+	return (index_count / 3) * (2 + 3 * vertex_groups) + 16;
 }
 
-void meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size)
+int meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size)
 {
 	using namespace meshopt;
 
 	assert(index_count % 3 == 0);
 
+	if (buffer_size <= index_count / 3 + 16)
+		return -1;
+
 	EdgeFifo edgefifo;
 	memset(edgefifo, -1, sizeof(edgefifo));
 
@@ -280,9 +298,13 @@ void meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, co
 
 	const unsigned char* code = buffer;
 	const unsigned char* data = buffer + index_count / 3;
+	const unsigned char* data_safe_end = buffer + buffer_size - 16;
 
 	for (size_t i = 0; i < index_count; i += 3)
 	{
+		if (data > data_safe_end)
+			return -2;
+
 		unsigned char codetri = *code++;
 
 		int fe = codetri >> 4;
@@ -359,6 +381,8 @@ void meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, co
 		}
 	}
 
-	assert(data == buffer + buffer_size);
-	(void)buffer_size;
+	if (data != data_safe_end)
+		return -3;
+
+	return 0;
 }

src/meshoptimizer.h

@@ -92,10 +92,11 @@ 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
  *
  * destination must contain enough space for the resulting index buffer (index_count elements)
  */
-MESHOPTIMIZER_API void meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size);
+MESHOPTIMIZER_API int meshopt_decodeIndexBuffer(unsigned int* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size);
 
 /**
  * Experimental: Mesh simplifier
@@ -282,11 +283,11 @@ inline size_t meshopt_encodeIndexBuffer(unsigned char* buffer, size_t buffer_siz
 }
 
 template <typename T>
-inline void meshopt_decodeIndexBuffer(T* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size)
+inline int meshopt_decodeIndexBuffer(T* destination, size_t index_count, const unsigned char* buffer, size_t buffer_size)
 {
 	meshopt_IndexAdapter<T> out(destination, 0, index_count);
 
-	meshopt_decodeIndexBuffer(out.data, index_count, buffer, buffer_size);
+	return meshopt_decodeIndexBuffer(out.data, index_count, buffer, buffer_size);
 }
 
 template <typename T>