From 7e9ce5a58bfa14ac9bd4e5a228a554fdd62e1e3e Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 5 Jul 2017 14:18:51 -0700 Subject: [PATCH] Don't overflow during AvatarData::toByteArray --- libraries/avatars/src/AvatarData.cpp | 44 ++++++++++++++++++++++++---- libraries/avatars/src/AvatarData.h | 23 +++++++++++++-- libraries/avatars/src/HeadData.cpp | 5 ++++ libraries/avatars/src/HeadData.h | 1 + 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 6eed23fb5b..6a0b668bfa 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -57,6 +57,27 @@ static const float DEFAULT_AVATAR_DENSITY = 1000.0f; // density of water #define ASSERT(COND) do { if (!(COND)) { abort(); } } while(0) +size_t AvatarDataPacket::maxFaceTrackerInfoSize(size_t numBlendshapeCoefficients) { + return FACE_TRACKER_INFO_SIZE + numBlendshapeCoefficients * sizeof(float); +} + +size_t AvatarDataPacket::maxJointDataSize(size_t numJoints) { + const size_t validityBitsSize = (size_t)std::ceil(numJoints / (float)BITS_IN_BYTE); + + size_t totalSize = sizeof(uint8_t); // numJoints + + totalSize += validityBitsSize; // Orientations mask + totalSize += numJoints * sizeof(SixByteQuat); // Orientations + totalSize += validityBitsSize; // Translations mask + totalSize += numJoints * sizeof(SixByteTrans); // Translations + + size_t NUM_FAUX_JOINT = 2; + totalSize += NUM_FAUX_JOINT * (sizeof(SixByteQuat) + sizeof(SixByteTrans)); // faux joints + + return totalSize; +} + + AvatarData::AvatarData() : SpatiallyNestable(NestableType::Avatar, QUuid()), _handPosition(0.0f), @@ -189,15 +210,11 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent lazyInitHeadData(); - QByteArray avatarDataByteArray(udt::MAX_PACKET_SIZE, 0); - unsigned char* destinationBuffer = reinterpret_cast(avatarDataByteArray.data()); - unsigned char* startPosition = destinationBuffer; - // special case, if we were asked for no data, then just include the flags all set to nothing if (dataDetail == NoData) { AvatarDataPacket::HasFlags packetStateFlags = 0; - memcpy(destinationBuffer, &packetStateFlags, sizeof(packetStateFlags)); - return avatarDataByteArray.left(sizeof(packetStateFlags)); + QByteArray avatarDataByteArray(reinterpret_cast(&packetStateFlags), sizeof(packetStateFlags)); + return avatarDataByteArray; } // FIXME - @@ -258,6 +275,15 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent hasJointData = sendAll || !sendMinimum; } + + const size_t byteArraySize = AvatarDataPacket::MAX_CONSTANT_HEADER_SIZE + + (hasFaceTrackerInfo ? AvatarDataPacket::maxFaceTrackerInfoSize(_headData->getNumSummedBlendshapeCoefficients()) : 0) + + (hasJointData ? AvatarDataPacket::maxJointDataSize(_jointData.size()) : 0); + + QByteArray avatarDataByteArray(byteArraySize, 0); + unsigned char* destinationBuffer = reinterpret_cast(avatarDataByteArray.data()); + unsigned char* startPosition = destinationBuffer; + // Leading flags, to indicate how much data is actually included in the packet... AvatarDataPacket::HasFlags packetStateFlags = (hasAvatarGlobalPosition ? AvatarDataPacket::PACKET_HAS_AVATAR_GLOBAL_POSITION : 0) @@ -624,6 +650,12 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent } int avatarDataSize = destinationBuffer - startPosition; + + if (avatarDataSize > byteArraySize) { + qCCritical(avatars) << "AvatarData::toByteArray buffer overflow"; // We've overflown into the heap + ASSERT(false); + } + return avatarDataByteArray.left(avatarDataSize); } // NOTE: This is never used in a "distanceAdjust" mode, so it's ok that it doesn't use a variable minimum rotation/translation diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index d6241c2c50..0ddf4e7d0a 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -140,6 +140,9 @@ namespace AvatarDataPacket { const HasFlags PACKET_HAS_JOINT_DATA = 1U << 11; const size_t AVATAR_HAS_FLAGS_SIZE = 2; + using SixByteQuat = uint8_t[6]; + using SixByteTrans = uint8_t[6]; + // NOTE: AvatarDataPackets start with a uint16_t sequence number that is not reflected in the Header structure. PACKED_BEGIN struct Header { @@ -158,8 +161,6 @@ namespace AvatarDataPacket { } PACKED_END; const size_t AVATAR_BOUNDING_BOX_SIZE = 24; - - using SixByteQuat = uint8_t[6]; PACKED_BEGIN struct AvatarOrientation { SixByteQuat avatarOrientation; // encodeded and compressed by packOrientationQuatToSixBytes() } PACKED_END; @@ -219,6 +220,21 @@ namespace AvatarDataPacket { } PACKED_END; const size_t AVATAR_LOCAL_POSITION_SIZE = 12; + const size_t MAX_CONSTANT_HEADER_SIZE = HEADER_SIZE + + AVATAR_GLOBAL_POSITION_SIZE + + AVATAR_BOUNDING_BOX_SIZE + + AVATAR_ORIENTATION_SIZE + + AVATAR_SCALE_SIZE + + LOOK_AT_POSITION_SIZE + + AUDIO_LOUDNESS_SIZE + + SENSOR_TO_WORLD_SIZE + + ADDITIONAL_FLAGS_SIZE + + PARENT_INFO_SIZE + + AVATAR_LOCAL_POSITION_SIZE; + + + // variable length structure follows + // only present if IS_FACE_TRACKER_CONNECTED flag is set in AvatarInfo.flags PACKED_BEGIN struct FaceTrackerInfo { float leftEyeBlink; @@ -229,8 +245,8 @@ namespace AvatarDataPacket { // float blendshapeCoefficients[numBlendshapeCoefficients]; } PACKED_END; const size_t FACE_TRACKER_INFO_SIZE = 17; + size_t maxFaceTrackerInfoSize(size_t numBlendshapeCoefficients); - // variable length structure follows /* struct JointData { uint8_t numJoints; @@ -240,6 +256,7 @@ namespace AvatarDataPacket { SixByteTrans translation[numValidTranslations]; // encodeded and compressed by packFloatVec3ToSignedTwoByteFixed() }; */ + size_t maxJointDataSize(size_t numJoints); } static const float MAX_AVATAR_SCALE = 1000.0f; diff --git a/libraries/avatars/src/HeadData.cpp b/libraries/avatars/src/HeadData.cpp index 8ae33a1b4f..1fda984e78 100644 --- a/libraries/avatars/src/HeadData.cpp +++ b/libraries/avatars/src/HeadData.cpp @@ -83,6 +83,11 @@ static const QMap& getBlendshapesLookupMap() { return blendshapeLookupMap; } +int HeadData::getNumSummedBlendshapeCoefficients() const { + int maxSize = std::max(_blendshapeCoefficients.size(), _transientBlendshapeCoefficients.size()); + return maxSize; +} + const QVector& HeadData::getSummedBlendshapeCoefficients() { int maxSize = std::max(_blendshapeCoefficients.size(), _transientBlendshapeCoefficients.size()); if (_summedBlendshapeCoefficients.size() != maxSize) { diff --git a/libraries/avatars/src/HeadData.h b/libraries/avatars/src/HeadData.h index c15714bd73..bcc2cacde5 100644 --- a/libraries/avatars/src/HeadData.h +++ b/libraries/avatars/src/HeadData.h @@ -57,6 +57,7 @@ public: void setBlendshape(QString name, float val); const QVector& getBlendshapeCoefficients() const { return _blendshapeCoefficients; } const QVector& getSummedBlendshapeCoefficients(); + int getNumSummedBlendshapeCoefficients() const; void setBlendshapeCoefficients(const QVector& blendshapeCoefficients) { _blendshapeCoefficients = blendshapeCoefficients; } const glm::vec3& getLookAtPosition() const { return _lookAtPosition; }