From 957991b67e76fb433354edf7ad4553000f4a7a93 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Thu, 4 Sep 2014 09:52:26 -0700 Subject: [PATCH] patch for possible static memory corruption on large edit entity messages --- .../entities/src/EntityEditPacketSender.cpp | 4 ++-- libraries/entities/src/EntityItemProperties.cpp | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index f66ef7ed68..f2588d0493 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -32,7 +32,7 @@ void EntityEditPacketSender::queueEditEntityMessage(PacketType type, EntityItemI } // use MAX_PACKET_SIZE since it's static and guaranteed to be larger than _maxPacketSize - static unsigned char bufferOut[MAX_PACKET_SIZE]; + unsigned char bufferOut[MAX_PACKET_SIZE]; int sizeOut = 0; if (EntityItemProperties::encodeEntityEditPacket(type, modelID, properties, &bufferOut[0], _maxPacketSize, sizeOut)) { @@ -45,7 +45,7 @@ void EntityEditPacketSender::queueEraseEntityMessage(const EntityItemID& entityI return; // bail early } // use MAX_PACKET_SIZE since it's static and guaranteed to be larger than _maxPacketSize - static unsigned char bufferOut[MAX_PACKET_SIZE]; + unsigned char bufferOut[MAX_PACKET_SIZE]; size_t sizeOut = 0; if (EntityItemProperties::encodeEraseEntityMessage(entityItemID, &bufferOut[0], _maxPacketSize, sizeOut)) { queueOctreeEditMessage(PacketTypeEntityErase, bufferOut, sizeOut); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 536dcea993..65babb6e16 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -596,8 +596,14 @@ bool EntityItemProperties::encodeEntityEditPacket(PacketType command, EntityItem packetData->endSubTree(); const unsigned char* finalizedData = packetData->getFinalizedData(); int finalizedSize = packetData->getFinalizedSize(); - memcpy(bufferOut, finalizedData, finalizedSize); - sizeOut = finalizedSize; + if (finalizedSize <= sizeIn) { + memcpy(bufferOut, finalizedData, finalizedSize); + sizeOut = finalizedSize; + } else { + qDebug() << "ERROR - encoded edit message doesn't fit in output buffer."; + sizeOut = 0; + success = false; + } } else { packetData->discardSubTree(); sizeOut = 0; @@ -747,8 +753,13 @@ bool EntityItemProperties::encodeEraseEntityMessage(const EntityItemID& entityIt unsigned char* outputBuffer, size_t maxLength, size_t& outputLength) { unsigned char* copyAt = outputBuffer; - uint16_t numberOfIds = 1; // only one entity ID in this message + + if (maxLength < sizeof(numberOfIds) + NUM_BYTES_RFC4122_UUID) { + qDebug() << "ERROR - encodeEraseEntityMessage() called with buffer that is too small!"; + outputLength = 0; + return false; + } memcpy(copyAt, &numberOfIds, sizeof(numberOfIds)); copyAt += sizeof(numberOfIds); outputLength = sizeof(numberOfIds);