3 files changed, 26 insertions, 7 deletions

diff --git a/ChangeLog.txt b/ChangeLog.txt
index c195870..22e9a0a 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -1,5 +1,11 @@
 2010-02-18  Jacek Antonelli  <jacek.antonelli@gmail.com>
 
+        * SNOW-492: LLDataPacker::unpackstring() is unsafe.
+          Patch by Robin Cornelius.
+
+          modified:   linden/indra/llmessage/lldatapacker.cpp
+
+
         * SNOW-488: Malformed animation crash.
           Patch by Robin Cornelius.
 
diff --git a/linden/indra/llcharacter/llkeyframemotion.cpp b/linden/indra/llcharacter/llkeyframemotion.cpp
index 46dee09..e6ef767 100644
--- a/linden/indra/llcharacter/llkeyframemotion.cpp
+++ b/linden/indra/llcharacter/llkeyframemotion.cpp
@@ -1355,8 +1355,8 @@ BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp)
                 }
                 else
                 {
-                        llwarns << "joint not found: " << joint_name << llendl;
-                        //return FALSE;
+                        llwarns << "joint not found: " << llendl;
+                        return FALSE;
                 }
 
                 joint_motion->mJointName = joint_name;
diff --git a/linden/indra/llmessage/lldatapacker.cpp b/linden/indra/llmessage/lldatapacker.cpp
index 1cdb475..e4243a5 100644
--- a/linden/indra/llmessage/lldatapacker.cpp
+++ b/linden/indra/llmessage/lldatapacker.cpp
@@ -186,18 +186,31 @@ BOOL LLDataPackerBinaryBuffer::packString(const std::string& value, const char *
         return success;
 }
 
-
 BOOL LLDataPackerBinaryBuffer::unpackString(std::string& value, const char *name)
 {
-        BOOL success = TRUE;
-        S32 length = (S32)strlen((char *)mCurBufferp) + 1; /*Flawfinder: ignore*/
+        //Sanitise the string before attemping ANY buffer operations
+        U8 * pos;
+        S32 length=0;
+        for(pos=mCurBufferp;pos<(mBufferp+mBufferSize);pos++)
+        {
+                length++;
+                if((*pos)==0)
+                        break;
+        }
 
-        success &= verifyLength(length, name);
+        if(length>=mBufferSize)
+        {
+                llwarns << "Unpack string failed, null termination not found"<<llendl;
+                return false;
+        }
+
+        if(!verifyLength(length, name))
+                return false;
 
         value = std::string((char*)mCurBufferp); // We already assume NULL termination calling strlen()
         
         mCurBufferp += length;
-        return success;
+        return true;
 }
 
 BOOL LLDataPackerBinaryBuffer::packBinaryData(const U8 *value, S32 size, const char *name)