Fix size error, lack of safety check, and type warning.

- Fixed a problem where the length of a different string was being
   used after concatenating a different one.
- Added in missing size checks to avoid buffer overruns.
- Corrected to size_t for proper tracking.
- Changed assumed remaining size to instead be compile-time safe.

Change-Id: I314effbdc14d725e5b16a5e655552c6a817cb3b2
Signed-off-by: Jon A. Cruz <jonc@osg.samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/2221
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>

diff --git a/resource/csdk/stack/src/oicgroup.c b/resource/csdk/stack/src/oicgroup.c
index 1ff11a8..3dd175d 100644 (file)
--- a/resource/csdk/stack/src/oicgroup.c
+++ b/resource/csdk/stack/src/oicgroup.c
@@ -731,8 +731,14 @@ exit:
 
 OCStackResult BuildStringFromActionSet(OCActionSet* actionset, char** desc)
 {
+    // Can't use the macros here as they are hardcoded to 'exit' and will
+    // result in dereferencing a null pointer.
+    if (!actionset || !desc)
+    {
+        return OC_STACK_INVALID_PARAM;
+    }
     char temp[1024] = { 0 };
-    int remaining = 1023;
+    size_t remaining = sizeof(temp) - 1;
     OCStackResult res = OC_STACK_ERROR;
 
     OCAction *action = actionset->head;
@@ -752,6 +758,11 @@ OCStackResult BuildStringFromActionSet(OCActionSet* actionset, char** desc)
 
     while (action != NULL)
     {
+        if (remaining < (strlen("uri=") + strlen(action->resourceUri) + 1))
+        {
+            res = OC_STACK_ERROR;
+            goto exit;
+        }
         strcat(temp, "uri=");
         remaining -= strlen("uri=");
         strcat(temp, action->resourceUri);
@@ -762,16 +773,28 @@ OCStackResult BuildStringFromActionSet(OCActionSet* actionset, char** desc)
         OCCapability *capas = action->head;
         while (capas != NULL)
         {
+            if (remaining < (strlen(capas->capability)
+                             + 1 + strlen(capas->status)))
+            {
+                res = OC_STACK_ERROR;
+                goto exit;
+            }
+
             strcat(temp, capas->capability);
             remaining -= strlen(capas->capability);
             strcat(temp, "=");
             remaining--;
             strcat(temp, capas->status);
-            remaining -= strlen(capas->capability);
+            remaining -= strlen(capas->status);
 
             capas = capas->next;
             if (capas != NULL)
             {
+                if (remaining < 1)
+                {
+                    res = OC_STACK_ERROR;
+                    goto exit;
+                }
                 strcat(temp, "|");
             }
         }
@@ -779,21 +802,24 @@ OCStackResult BuildStringFromActionSet(OCActionSet* actionset, char** desc)
         action = action->next;
         if (action != NULL)
         {
+            if (remaining < strlen(ACTION_DELIMITER))
+            {
+                res = OC_STACK_ERROR;
+                goto exit;
+            }
             strcat(temp, ACTION_DELIMITER);
             remaining--;
         }
     }
 
-    *desc = (char *) OICMalloc(1024 - remaining);
+    *desc = OICStrdup(temp);
     VARIFY_POINTER_NULL(*desc, res, exit);
-    strcpy(*desc, temp);
 
     return OC_STACK_OK;
 
 exit:
     OCFREE(*desc);
     return res;
-
 }
 
 OCStackApplicationResult ActionSetCB(void* context, OCDoHandle handle,