From ce139f26e0542e3ab4c2334c10044a2af9085049 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev <117672381+akleshchev@users.noreply.github.com> Date: Wed, 4 Mar 2026 22:39:47 +0200 Subject: #5486 Fix potential unpackBinaryData buffer issues --- indra/llmessage/lldatapacker.cpp | 67 +++++++++++++++++++++++++++++++--------- indra/llmessage/lldatapacker.h | 8 ++--- 2 files changed, 57 insertions(+), 18 deletions(-) (limited to 'indra/llmessage') diff --git a/indra/llmessage/lldatapacker.cpp b/indra/llmessage/lldatapacker.cpp index e911150787..ecd0b4ee8d 100644 --- a/indra/llmessage/lldatapacker.cpp +++ b/indra/llmessage/lldatapacker.cpp @@ -289,32 +289,46 @@ bool LLDataPackerBinaryBuffer::packBinaryData(const U8 *value, S32 size, const c } -bool LLDataPackerBinaryBuffer::unpackBinaryData(U8 *value, S32 &size, const char *name) +bool LLDataPackerBinaryBuffer::unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name) { if (!verifyLength(4, name)) { LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData would unpack invalid data, aborting!" << LL_ENDL; + out_size = 0; return false; } - htolememcpy(&size, mCurBufferp, MVT_S32, 4); + if (value_size < 0) + { + LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData passed negative buffer size, aborting!" << LL_ENDL; + out_size = 0; + return false; + } + + htolememcpy(&out_size, mCurBufferp, MVT_S32, 4); - if (size < 0) + if (out_size < 0) { LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData unpacked invalid size, aborting!" << LL_ENDL; + out_size = 0; return false; } mCurBufferp += 4; - if (!verifyLength(size, name)) + if (!verifyLength(out_size, name)) { LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData would unpack invalid data, aborting!" << LL_ENDL; return false; } + S32 copy_size = llmin(out_size, value_size); + htolememcpy(value, mCurBufferp, MVT_VARIABLE, copy_size); + mCurBufferp += out_size; - htolememcpy(value, mCurBufferp, MVT_VARIABLE, size); - mCurBufferp += size; + if (value_size < out_size) + { + LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData buffer too small for data, truncating!" << LL_ENDL; + } return true; } @@ -836,21 +850,34 @@ bool LLDataPackerAsciiBuffer::packBinaryData(const U8 *value, S32 size, const ch } -bool LLDataPackerAsciiBuffer::unpackBinaryData(U8 *value, S32 &size, const char *name) +bool LLDataPackerAsciiBuffer::unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name) { bool success = true; char valuestr[DP_BUFSIZE]; /* Flawfinder: ignore */ if (!getValueStr(name, valuestr, DP_BUFSIZE)) { + out_size = 0; + return false; + } + + if (value_size < 0) + { + LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData passed negative buffer size, aborting!" << LL_ENDL; + out_size = 0; return false; } char *cur_pos = &valuestr[0]; - sscanf(valuestr,"%010d", &size); + sscanf(valuestr,"%010d", &out_size); cur_pos += 11; + S32 max_bytes = llmin(out_size, value_size); + if (max_bytes != out_size) + { + LL_WARNS() << "LLDataPackerAsciiBuffer::unpackBinaryData: buffer too small for data, truncating!" << LL_ENDL; + } S32 i; - for (i = 0; i < size; i++) + for (i = 0; i < max_bytes; i++) { S32 val; sscanf(cur_pos,"%02x", &val); @@ -1634,28 +1661,40 @@ bool LLDataPackerAsciiFile::packBinaryData(const U8 *value, S32 size, const char } -bool LLDataPackerAsciiFile::unpackBinaryData(U8 *value, S32 &size, const char *name) +bool LLDataPackerAsciiFile::unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name) { - bool success = true; char valuestr[DP_BUFSIZE]; /*Flawfinder: ignore*/ if (!getValueStr(name, valuestr, DP_BUFSIZE)) { + out_size = 0; + return false; + } + + if (value_size < 0) + { + LL_WARNS() << "LLDataPackerBinaryBuffer::unpackBinaryData passed negative buffer size, aborting!" << LL_ENDL; + out_size = 0; return false; } char *cur_pos = &valuestr[0]; - sscanf(valuestr,"%010d", &size); + sscanf(valuestr,"%010d", &out_size); cur_pos += 11; + S32 max_bytes = llmin(out_size, value_size); + if (max_bytes != out_size) + { + LL_WARNS() << "LLDataPackerAsciiBuffer::unpackBinaryData: buffer too small for data, truncating!" << LL_ENDL; + } S32 i; - for (i = 0; i < size; i++) + for (i = 0; i < max_bytes; i++) { S32 val; sscanf(cur_pos,"%02x", &val); value[i] = val; cur_pos += 3; } - return success; + return true; } diff --git a/indra/llmessage/lldatapacker.h b/indra/llmessage/lldatapacker.h index 167c102b43..5ac45356cf 100644 --- a/indra/llmessage/lldatapacker.h +++ b/indra/llmessage/lldatapacker.h @@ -49,7 +49,7 @@ public: virtual bool unpackString(std::string& value, const char *name) = 0; virtual bool packBinaryData(const U8 *value, S32 size, const char *name) = 0; - virtual bool unpackBinaryData(U8 *value, S32 &size, const char *name) = 0; + virtual bool unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name) = 0; // Constant size binary data packing virtual bool packBinaryDataFixed(const U8 *value, S32 size, const char *name) = 0; @@ -135,7 +135,7 @@ public: /*virtual*/ bool unpackString(std::string& value, const char *name); /*virtual*/ bool packBinaryData(const U8 *value, S32 size, const char *name); - /*virtual*/ bool unpackBinaryData(U8 *value, S32 &size, const char *name); + /*virtual*/ bool unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name); // Constant size binary data packing /*virtual*/ bool packBinaryDataFixed(const U8 *value, S32 size, const char *name); @@ -246,7 +246,7 @@ public: /*virtual*/ bool unpackString(std::string& value, const char *name); /*virtual*/ bool packBinaryData(const U8 *value, S32 size, const char *name); - /*virtual*/ bool unpackBinaryData(U8 *value, S32 &size, const char *name); + /*virtual*/ bool unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name); // Constant size binary data packing /*virtual*/ bool packBinaryDataFixed(const U8 *value, S32 size, const char *name); @@ -378,7 +378,7 @@ public: /*virtual*/ bool unpackString(std::string& value, const char *name); /*virtual*/ bool packBinaryData(const U8 *value, S32 size, const char *name); - /*virtual*/ bool unpackBinaryData(U8 *value, S32 &size, const char *name); + /*virtual*/ bool unpackBinaryData(U8 *value, S32 value_size, S32 &out_size, const char *name); /*virtual*/ bool packBinaryDataFixed(const U8 *value, S32 size, const char *name); /*virtual*/ bool unpackBinaryDataFixed(U8 *value, S32 size, const char *name); -- cgit v1.3 From 5b485bdc6046867e8b03906e8bdd0f02471f9fa8 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev <117672381+akleshchev@users.noreply.github.com> Date: Fri, 3 Apr 2026 02:11:15 +0300 Subject: #5612 Improve profiling coverage --- indra/llmessage/llassetstorage.cpp | 3 ++- indra/newview/llfloaterinspect.cpp | 1 + indra/newview/llremoteparcelrequest.cpp | 1 + indra/newview/llviewergenericmessage.cpp | 3 +++ indra/newview/llviewermessage.cpp | 8 ++++++++ indra/newview/llviewerobject.cpp | 1 + indra/newview/llvoavatar.cpp | 1 + indra/newview/llworld.cpp | 1 + indra/newview/llworldmapmessage.cpp | 2 ++ 9 files changed, 20 insertions(+), 1 deletion(-) (limited to 'indra/llmessage') diff --git a/indra/llmessage/llassetstorage.cpp b/indra/llmessage/llassetstorage.cpp index 4c3acb27f4..b6a98575f9 100644 --- a/indra/llmessage/llassetstorage.cpp +++ b/indra/llmessage/llassetstorage.cpp @@ -453,6 +453,7 @@ bool LLAssetStorage::findInCacheAndInvokeCallback(const LLUUID& uuid, LLAssetTyp bool exists = LLFileSystem::getExists(uuid, type); if (exists) { + LL_PROFILE_ZONE_SCOPED; LLFileSystem file(uuid, type); U32 size = file.getSize(); if (size > 0) @@ -562,7 +563,7 @@ void LLAssetStorage::getAssetData(const LLUUID uuid, if (callback == tmp->mDownCallback && user_data == tmp->mUserData) { // this is a duplicate from the same subsystem - throw it away - LL_WARNS("AssetStorage") << "Discarding duplicate request for asset " << uuid + LL_DEBUGS("AssetStorage") << "Discarding duplicate request for asset " << uuid << "." << LLAssetType::lookup(type) << LL_ENDL; return; } diff --git a/indra/newview/llfloaterinspect.cpp b/indra/newview/llfloaterinspect.cpp index c0fe7ad896..163edf0426 100644 --- a/indra/newview/llfloaterinspect.cpp +++ b/indra/newview/llfloaterinspect.cpp @@ -100,6 +100,7 @@ void LLFloaterInspect::onOpen(const LLSD& key) LLSelectMgr::getInstance()->setForceSelection(forcesel); // restore previouis value mObjectSelection = LLSelectMgr::getInstance()->getSelection(); refresh(); + mDirty = false; } void LLFloaterInspect::onClickCreatorProfile() { diff --git a/indra/newview/llremoteparcelrequest.cpp b/indra/newview/llremoteparcelrequest.cpp index f89afd38ab..c1b33f313b 100644 --- a/indra/newview/llremoteparcelrequest.cpp +++ b/indra/newview/llremoteparcelrequest.cpp @@ -84,6 +84,7 @@ void LLRemoteParcelInfoProcessor::removeObserver(const LLUUID& parcel_id, LLRemo //static void LLRemoteParcelInfoProcessor::processParcelInfoReply(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLParcelData parcel_data; msg->getUUID ("Data", "ParcelID", parcel_data.parcel_id); diff --git a/indra/newview/llviewergenericmessage.cpp b/indra/newview/llviewergenericmessage.cpp index fd894a5997..f3a0f026b8 100644 --- a/indra/newview/llviewergenericmessage.cpp +++ b/indra/newview/llviewergenericmessage.cpp @@ -73,6 +73,7 @@ void send_generic_message(const std::string& method, void process_generic_message(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID agent_id; msg->getUUID("AgentData", "AgentID", agent_id); if (agent_id != gAgent.getID()) @@ -95,6 +96,7 @@ void process_generic_message(LLMessageSystem* msg, void**) void process_generic_streaming_message(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLGenericStreamingMessage data; data.unpack(msg); switch (data.mMethod) @@ -110,6 +112,7 @@ void process_generic_streaming_message(LLMessageSystem* msg, void**) void process_large_generic_message(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID agent_id; msg->getUUID("AgentData", "AgentID", agent_id); if (agent_id != gAgent.getID()) diff --git a/indra/newview/llviewermessage.cpp b/indra/newview/llviewermessage.cpp index 5d8bd45218..812ba76551 100644 --- a/indra/newview/llviewermessage.cpp +++ b/indra/newview/llviewermessage.cpp @@ -3554,6 +3554,7 @@ extern U32Bits gObjectData; void process_object_update(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; // Update the data counters if (mesgsys->getReceiveCompressedSize()) { @@ -3575,6 +3576,7 @@ void process_object_update(LLMessageSystem *mesgsys, void **user_data) void process_compressed_object_update(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; // Update the data counters if (mesgsys->getReceiveCompressedSize()) { @@ -3596,6 +3598,7 @@ void process_compressed_object_update(LLMessageSystem *mesgsys, void **user_data void process_cached_object_update(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; // Update the data counters if (mesgsys->getReceiveCompressedSize()) { @@ -3613,6 +3616,7 @@ void process_cached_object_update(LLMessageSystem *mesgsys, void **user_data) void process_terse_object_update_improved(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; if (mesgsys->getReceiveCompressedSize()) { gObjectData += (U32Bytes)mesgsys->getReceiveCompressedSize(); @@ -3972,6 +3976,7 @@ void process_sim_stats(LLMessageSystem *msg, void **user_data) void process_avatar_animation(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID animation_id; LLUUID uuid; S32 anim_sequence_id; @@ -4083,6 +4088,7 @@ void process_avatar_animation(LLMessageSystem *mesgsys, void **user_data) void process_object_animation(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID animation_id; LLUUID uuid; S32 anim_sequence_id; @@ -4148,6 +4154,7 @@ void process_object_animation(LLMessageSystem *mesgsys, void **user_data) void process_avatar_appearance(LLMessageSystem *mesgsys, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID uuid; mesgsys->getUUIDFast(_PREHASH_Sender, _PREHASH_ID, uuid); @@ -5679,6 +5686,7 @@ void process_script_experience_details(const LLSD& experience_details, LLSD args void process_script_question(LLMessageSystem *msg, void **user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; // *TODO: Translate owner name -> [FIRST] [LAST] LLHost sender = msg->getSender(); diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp index 521c6731e0..7c26cb3c9f 100644 --- a/indra/newview/llviewerobject.cpp +++ b/indra/newview/llviewerobject.cpp @@ -3210,6 +3210,7 @@ S32 LLFilenameAndTask::sCount = 0; // static void LLViewerObject::processTaskInv(LLMessageSystem* msg, void** user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLUUID task_id; msg->getUUIDFast(_PREHASH_InventoryData, _PREHASH_TaskID, task_id); LLViewerObject* object = gObjectList.findObject(task_id); diff --git a/indra/newview/llvoavatar.cpp b/indra/newview/llvoavatar.cpp index bb152ced40..2f39a76156 100644 --- a/indra/newview/llvoavatar.cpp +++ b/indra/newview/llvoavatar.cpp @@ -5984,6 +5984,7 @@ const LLUUID& LLVOAvatar::getStepSound() const //----------------------------------------------------------------------------- void LLVOAvatar::processAnimationStateChanges() { + LL_PROFILE_ZONE_SCOPED_CATEGORY_AVATAR; if ( isAnyAnimationSignaled(AGENT_WALK_ANIMS, NUM_AGENT_WALK_ANIMS) ) { startMotion(ANIM_AGENT_WALK_ADJUST); diff --git a/indra/newview/llworld.cpp b/indra/newview/llworld.cpp index 47e1815bc2..d02694de7d 100644 --- a/indra/newview/llworld.cpp +++ b/indra/newview/llworld.cpp @@ -1217,6 +1217,7 @@ void process_disable_simulator(LLMessageSystem *mesgsys, void **user_data) void process_region_handshake(LLMessageSystem* msg, void** user_data) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; LLHost host = msg->getSender(); LLViewerRegion* regionp = LLWorld::getInstance()->getRegion(host); if (!regionp) diff --git a/indra/newview/llworldmapmessage.cpp b/indra/newview/llworldmapmessage.cpp index 3264f8ae8b..c039f9de3f 100644 --- a/indra/newview/llworldmapmessage.cpp +++ b/indra/newview/llworldmapmessage.cpp @@ -154,6 +154,7 @@ void LLWorldMapMessage::sendMapBlockRequest(U16 min_x, U16 min_y, U16 max_x, U16 // public static void LLWorldMapMessage::processMapBlockReply(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; if (gNonInteractive) { return; @@ -248,6 +249,7 @@ void LLWorldMapMessage::processMapBlockReply(LLMessageSystem* msg, void**) // public static void LLWorldMapMessage::processMapItemReply(LLMessageSystem* msg, void**) { + LL_PROFILE_ZONE_SCOPED_CATEGORY_NETWORK; //LL_INFOS("WorldMap") << LL_ENDL; U32 type; msg->getU32Fast(_PREHASH_RequestData, _PREHASH_ItemType, type); -- cgit v1.3