[IOT-1611] Memory corruption in OCSetAttribute

Fix memory corruption bugs in OCSetAttribute introduced by
https://gerrit.iotivity.org/gerrit/#/c/14377/ which left pointers to
freed memory in the attribute list, which was then later used.

Change-Id: Ib776d4bd14aab7b39c74188246005628fbe18ecf
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/14873
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
Reviewed-by: Phil Coval <philippe.coval@osg.samsung.com>
Reviewed-by: Habib Virji <habib.virji@samsung.com>
Reviewed-by: Ziran Sun <ziran.sun@samsung.com>
(cherry picked from commit f4217dab180e728271e79d5a3ea314c39fb1d628)
Reviewed-on: https://gerrit.iotivity.org/gerrit/14905
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>

diff --git a/resource/csdk/stack/src/ocresource.c b/resource/csdk/stack/src/ocresource.c
index 814a8b8..0a0b126 100755 (executable)
--- a/resource/csdk/stack/src/ocresource.c
+++ b/resource/csdk/stack/src/ocresource.c
@@ -1565,45 +1565,47 @@ OCStackResult OCGetPropertyValue(OCPayloadType type, const char *prop, void **va
 
 OCStackResult OCSetAttribute(OCResource* resource, const char* attribute, const void* value)
 {
-    OCAttribute *resAttrib = (OCAttribute *)OICCalloc(1, sizeof(OCAttribute));
-    VERIFY_PARAM_NON_NULL(TAG, resAttrib, "Failed allocation OCAttribute");
-    resAttrib->attrName = OICStrdup(attribute);
-    VERIFY_PARAM_NON_NULL(TAG, resAttrib->attrName, "Failed allocating attribute name");
-    // A special case when value is of type OCStringLL
-    if (0 == strcmp(OC_RSRVD_DATA_MODEL_VERSION, attribute))
+    // See if the attribute already exists in the list.
+    OCAttribute *resAttrib;
+    for (resAttrib = resource->rsrcAttributes; resAttrib; resAttrib = resAttrib->next)
     {
-        resAttrib->attrValue = CloneOCStringLL((OCStringLL *)value);
+        if (0 == strcmp(attribute, resAttrib->attrName))
+        {
+            // Found, free the old value.
+            if (0 == strcmp(OC_RSRVD_DATA_MODEL_VERSION, resAttrib->attrName))
+            {
+                OCFreeOCStringLL((OCStringLL *)resAttrib->attrValue);
+            }
+            else
+            {
+                OICFree((char *)resAttrib->attrValue);
+            }
+            break;
+        }
     }
-    else
+
+    // If not already in the list, add it.
+    if (NULL == resAttrib)
     {
-        resAttrib->attrValue = OICStrdup((char *)value);
+        resAttrib = (OCAttribute *)OICCalloc(1, sizeof(OCAttribute));
+        VERIFY_PARAM_NON_NULL(TAG, resAttrib, "Failed allocating OCAttribute");
+        resAttrib->attrName = OICStrdup(attribute);
+        VERIFY_PARAM_NON_NULL(TAG, resAttrib->attrName, "Failed allocating attribute name");
+        resAttrib->next = resource->rsrcAttributes;
+        resource->rsrcAttributes = resAttrib;
     }
-    VERIFY_PARAM_NON_NULL(TAG, resAttrib->attrValue, "Failed allocating attribute value");
-    resAttrib->next = NULL;
 
-    if (!resource->rsrcAttributes)
+    // Fill in the new value.
+    if (0 == strcmp(OC_RSRVD_DATA_MODEL_VERSION, attribute))
     {
-        resource->rsrcAttributes = resAttrib;
+        resAttrib->attrValue = CloneOCStringLL((OCStringLL *)value);
     }
     else
     {
-        OCAttribute *temp = resource->rsrcAttributes;
-        for (; temp->next; temp = temp->next)
-        {
-            if (0 == strcmp(attribute, temp->attrName))
-            {
-                if (0 == strcmp(OC_RSRVD_DATA_MODEL_VERSION, temp->attrName))
-                {
-                    OCFreeOCStringLL((OCStringLL *)temp->attrValue);
-                }
-                 {
-                    OICFree((char *)temp->attrValue);
-                }
-                break;
-            }
-        }
-        temp->next = resAttrib;
+        resAttrib->attrValue = OICStrdup((char *)value);
     }
+    VERIFY_PARAM_NON_NULL(TAG, resAttrib->attrValue, "Failed allocating attribute value");
+
     return OC_STACK_OK;
 
 exit: