[IOT-2117][Bug][Data loss] No check for snprintf return.
authorSenthil Kumar G S <senthil.gs@samsung.com>
Thu, 11 May 2017 12:09:31 +0000 (17:39 +0530)
committerUze Choi <uzchoi@samsung.com>
Tue, 16 May 2017 00:48:00 +0000 (00:48 +0000)
Added code to properly handle the return value of snprintf().

Change-Id: I12bb53e32279d0ac7c09740389e6d3124b19c4dc
Signed-off-by: Senthil Kumar G S <senthil.gs@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/19657
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Phil Coval <philippe.coval@osg.samsung.com>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
resource/csdk/stack/src/ocendpoint.c
resource/csdk/stack/src/ocpayloadconvert.c

index 8f4e442..d70d41a 100644 (file)
@@ -29,6 +29,9 @@
 #define VERIFY_GT_ZERO(arg) { if (arg < 1) {OIC_LOG(FATAL, TAG, #arg " < 1"); goto exit;} }
 #define VERIFY_GT(arg1, arg2) { if (arg1 <= arg2) {OIC_LOG(FATAL, TAG, #arg1 " <= " #arg2); goto exit;} }
 #define VERIFY_LT_OR_EQ(arg1, arg2) { if (arg1 > arg2) {OIC_LOG(FATAL, TAG, #arg1 " > " #arg2); goto exit;} }
+#define VERIFY_SNPRINTF_RET(arg1, arg2) \
+    { if (0 > arg1 || arg1 >= arg2) {OIC_LOG(FATAL, TAG, "Error (snprintf)"); goto exit;} } \
+
 #define TAG  "OIC_RI_ENDPOINT"
 
 OCStackResult OCGetSupportedEndpointFlags(const OCTpsSchemeFlags givenFlags, OCTpsSchemeFlags* out)
@@ -246,32 +249,36 @@ char* OCCreateEndpointString(const OCEndpointPayload* endpoint)
         if (endpoint->family & OC_IP_USE_V4)
         {
             // ipv4
-            snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s:%d", endpoint->tps,
-                     endpoint->addr, endpoint->port);
+            int snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s:%d", endpoint->tps,
+                                  endpoint->addr, endpoint->port);
+            VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
         }
         else
         {
             // ipv6
-            snprintf(buf, MAX_ADDR_STR_SIZE, "%s://[%s]:%d", endpoint->tps,
-                     endpoint->addr, endpoint->port);
+            int snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://[%s]:%d", endpoint->tps,
+                                  endpoint->addr, endpoint->port);
+            VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
         }
     }
 #ifdef EDR_ADAPTER
     else if ((strcmp(endpoint->tps, COAP_RFCOMM_STR) == 0))
     {
         // coap+rfcomm
-        snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s",
-                 endpoint->tps, endpoint->addr);
+        int snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s",
+                              endpoint->tps, endpoint->addr);
+        VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
     }
 #endif
     else
     {
         OIC_LOG_V(ERROR, TAG, "Payload has invalid TPS!!! %s", endpoint->tps);
-        return NULL;
+        goto exit;
     }
     return buf;
 
 exit:
+    OICFree(buf);
     return NULL;
 }
 
@@ -282,6 +289,7 @@ char* OCCreateEndpointStringFromCA(const CAEndpoint_t* endpoint)
         return NULL;
     }
 
+    int snRet = -1;
     char *buf = NULL;
     OCTpsSchemeFlags tps = OC_NO_TPS;
     OCStackResult result = OCGetMatchedTpsFlags(endpoint->adapter, endpoint->flags, &tps);
@@ -310,20 +318,23 @@ char* OCCreateEndpointStringFromCA(const CAEndpoint_t* endpoint)
         if (endpoint->flags & CA_IPV4)
         {
             // ipv4
-            snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s:%d", ConvertTpsToString(tps),
-                     endpoint->addr, endpoint->port);
+            snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s:%d", ConvertTpsToString(tps),
+                                  endpoint->addr, endpoint->port);
+            VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
         }
         else
         {
             // ipv6
-            snprintf(buf, MAX_ADDR_STR_SIZE, "%s://[%s]:%d", ConvertTpsToString(tps),
-                     endpoint->addr, endpoint->port);
+            snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://[%s]:%d", ConvertTpsToString(tps),
+                                  endpoint->addr, endpoint->port);
+            VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
         }
         break;
 #ifdef EDR_ADAPTER
     case OC_COAP_RFCOMM:
         // coap+rfcomm
-        snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s", ConvertTpsToString(tps), endpoint->addr);
+        snRet = snprintf(buf, MAX_ADDR_STR_SIZE, "%s://%s", ConvertTpsToString(tps), endpoint->addr);
+        VERIFY_SNPRINTF_RET(snRet, MAX_ADDR_STR_SIZE);
         break;
 #endif
     default:
index 41d3b81..b77dac5 100755 (executable)
@@ -165,7 +165,7 @@ static int64_t OCConvertPayloadHelper(OCPayload* payload, OCPayloadFormat format
         case PAYLOAD_TYPE_SECURITY:
             return OCConvertSecurityPayload((OCSecurityPayload*)payload, outPayload, size);
         case PAYLOAD_TYPE_INTROSPECTION:
-            return OCConvertIntrospectionPayload((OCIntrospectionPayload*)payload, 
+            return OCConvertIntrospectionPayload((OCIntrospectionPayload*)payload,
                                                  outPayload, size);
         default:
             OIC_LOG_V(INFO, TAG, "ConvertPayload default %d", payload->type);
@@ -201,7 +201,7 @@ static int64_t OCConvertSecurityPayload(OCSecurityPayload* payload, uint8_t* out
     return CborNoError;
 }
 
-static int64_t OCConvertIntrospectionPayload(OCIntrospectionPayload *payload, 
+static int64_t OCConvertIntrospectionPayload(OCIntrospectionPayload *payload,
         uint8_t *outPayload, size_t *size)
 {
     memcpy(outPayload, payload->cborPayload.bytes, payload->cborPayload.len);
@@ -262,7 +262,13 @@ static int64_t OCConvertResourcePayloadCbor(CborEncoder *linkArray, OCResourcePa
         }
         VERIFY_CBOR_SUCCESS(TAG, err, "Failed creating endpoint string");
         char uri[MAX_URI_LENGTH];
-        snprintf(uri, MAX_URI_LENGTH, "%s%s", endpointStr, resource->uri);
+        int snRet = snprintf(uri, MAX_URI_LENGTH, "%s%s", endpointStr, resource->uri);
+        if(0 > snRet || snRet >= MAX_URI_LENGTH)
+        {
+            OICFree(endpointStr);
+            VERIFY_CBOR_SUCCESS(TAG, CborErrorInternalError, "Error (snprintf)");
+        }
+
         OICFree(endpointStr);
 
         err |= AddTextStringToMap(&linkMap, OC_RSRVD_HREF, sizeof(OC_RSRVD_HREF) - 1,
@@ -273,7 +279,11 @@ static int64_t OCConvertResourcePayloadCbor(CborEncoder *linkArray, OCResourcePa
              resource->anchor)
     {
         char uri[MAX_URI_LENGTH];
-        snprintf(uri, MAX_URI_LENGTH, "%s%s", resource->anchor, resource->uri);
+        int snRet = snprintf(uri, MAX_URI_LENGTH, "%s%s", resource->anchor, resource->uri);
+        if(0 > snRet || snRet >= MAX_URI_LENGTH)
+        {
+            VERIFY_CBOR_SUCCESS(TAG, CborErrorInternalError, "Error (snprintf)");
+        }
 
         err |= AddTextStringToMap(&linkMap, OC_RSRVD_HREF, sizeof(OC_RSRVD_HREF) - 1,
                                   uri);
@@ -574,7 +584,11 @@ static int64_t OCConvertDiscoveryPayloadVndOcfCbor(OCDiscoveryPayload *payload,
                 resource->rel && !strcmp(resource->rel, "self"))
             {
                 char uri[MAX_URI_LENGTH];
-                snprintf(uri, MAX_URI_LENGTH, "ocf://%s%s", payload->sid, resource->uri);
+                int snRet = snprintf(uri, MAX_URI_LENGTH, "ocf://%s%s", payload->sid, resource->uri);
+                if(0 > snRet || snRet >= MAX_URI_LENGTH)
+                {
+                    VERIFY_CBOR_SUCCESS(TAG, CborErrorInternalError, "Error (snprintf)");
+                }
 
                 err |= AddTextStringToMap(&linkMap, OC_RSRVD_HREF, sizeof(OC_RSRVD_HREF) - 1,
                                           uri);
@@ -593,7 +607,12 @@ static int64_t OCConvertDiscoveryPayloadVndOcfCbor(OCDiscoveryPayload *payload,
 
             // Anchor
             char anchor[MAX_URI_LENGTH];
-            snprintf(anchor, MAX_URI_LENGTH, "ocf://%s", payload->sid);
+            int snRet = snprintf(anchor, MAX_URI_LENGTH, "ocf://%s", payload->sid);
+            if(0 > snRet || snRet >= MAX_URI_LENGTH)
+            {
+                VERIFY_CBOR_SUCCESS(TAG, CborErrorInternalError, "Error (snprintf)");
+            }
+
             err |= AddTextStringToMap(&linkMap, OC_RSRVD_URI, sizeof(OC_RSRVD_URI) - 1, anchor);
             VERIFY_CBOR_SUCCESS(TAG, err, "Failed adding anchor to links map");