static analysis: buffer size violations 63/24163/7
authorMats Wichmann <mats@linux.com>
Thu, 15 Feb 2018 20:15:35 +0000 (13:15 -0700)
committerMats Wichmann <mats@linux.com>
Wed, 18 Apr 2018 13:34:35 +0000 (13:34 +0000)
fix reported "might leave destination string unterminated" when
using strncpy and the size argument is the size of the desitnation
buffer. There are several ways to fix, but the IoTivity API OCStrncpy
adjusts the size before calling the underlying fuction and so fixes
the problem. This is the coding standard recommendation anyway.

for instances in examples, oic_string.h is not in the public API,
so just fix up the counts.

bridging/common/messageHandler.cpp had a third strncpy which was not
flagged - but it was using a constant that did not match the size
in the destination - MPMResourceList.href is size MPM_MAX_URI_LEN,
so this was adjusted.

service/notification/src/consumer/NSConsumerCommon.c had this construct:
   sizeof(char) * NS_DEVICE_ID_LENGTH
in several places (one of which was one of these strncpy calls that
was changed). the instances were shortened NS_DEVICE_ID_LENGTH for
readability. This is not fixing any reported problem and can be
dropped if it bothers reviewers.

Change-Id: I8f22f7dd704849477dad0dd1f16cd9276ebf1d04
Signed-off-by: Mats Wichmann <mats@linux.com>
bridging/common/messageHandler.cpp
resource/csdk/security/provisioning/sample/certgenerator.cpp
resource/csdk/stack/samples/linux/secure/occlientbasicops.cpp
service/coap-http-proxy/samples/proxy_client.c
service/notification/src/consumer/NSConsumerCommon.c
service/notification/src/consumer/NSConsumerCommunication.c
service/notification/unittest/NSConsumerTest2.cpp
service/notification/unittest/NSProviderTest2.cpp

index 1fd7f13..249fbec 100644 (file)
@@ -30,6 +30,7 @@
 #include <unistd.h>
 #endif
 #include "oic_malloc.h"
+#include "oic_string.h"
 #include "pluginIf.h"
 #include "pluginServer.h"
 #include "cbor.h"
@@ -315,7 +316,7 @@ void MPMParseMetaData(const uint8_t *buff, size_t size, MPMResourceList **list,
 
                 err = cbor_value_dup_text_string(&curVal, &input, &len, NULL);
                 VERIFY_CBOR_SUCCESS_OR_OUT_OF_MEMORY(TAG, err, " Copying Text string");
-                strncpy(tempPtr->href, input, MPM_MAX_LENGTH_64);
+                OICStrcpy(tempPtr->href, MPM_MAX_URI_LEN, input);
                 OIC_LOG_V(DEBUG, TAG, "\"ref\":%s\n", input);
                 free(input);
                 input = NULL;
@@ -324,7 +325,7 @@ void MPMParseMetaData(const uint8_t *buff, size_t size, MPMResourceList **list,
                 VERIFY_CBOR_SUCCESS_OR_OUT_OF_MEMORY(TAG, err, " Finding Rt in link map ");
                 err = cbor_value_dup_text_string(&curVal, &input, &len, NULL);
                 VERIFY_CBOR_SUCCESS_OR_OUT_OF_MEMORY(TAG, err, " Copying Text string");
-                strncpy(tempPtr->rt, input, MPM_MAX_LENGTH_64);
+                OICStrcpy(tempPtr->rt, MPM_MAX_LENGTH_64, input);
                 OIC_LOG_V(DEBUG, TAG, "\"rt\":%s\n", input);
                 free(input);
                 input = NULL;
@@ -333,7 +334,7 @@ void MPMParseMetaData(const uint8_t *buff, size_t size, MPMResourceList **list,
                 VERIFY_CBOR_SUCCESS_OR_OUT_OF_MEMORY(TAG, err, " Finding If's in link map ");
                 err = cbor_value_dup_text_string(&curVal, &input, &len, NULL);
                 VERIFY_CBOR_SUCCESS_OR_OUT_OF_MEMORY(TAG, err, " Copying Text string");
-                strncpy(tempPtr->interfaces, input, MPM_MAX_LENGTH_64);
+                OICStrcpy(tempPtr->interfaces, MPM_MAX_LENGTH_64, input);
                 OIC_LOG_V(DEBUG, TAG, "\"if\":%s\n", input);
                 free(input);
                 input = NULL;
index 85e31e1..25fa031 100644 (file)
@@ -139,7 +139,7 @@ static void DoGenCertificate(CertType certType)
     }
     else
     {
-        strncpy(issKeyPairName, subjKeyPairName, sizeof(subjKeyPairName));
+        strncpy(issKeyPairName, subjKeyPairName, sizeof(subjKeyPairName) - 1);
     }
 
     // -- Load public key --
index 3f59797..f19b024 100644 (file)
@@ -41,7 +41,6 @@
 #include "occlientbasicops.h"
 #include "ocpayload.h"
 #include "experimental/payload_logging.h"
-#include "oic_string.h"
 #include "common.h"
 
 #define TAG "occlientbasicops"
@@ -501,7 +500,7 @@ int parseClientResponse(OCClientResponse *clientResponse)
 #ifdef __WITH_TLS__
                     if (WithTcp && 0 == strcmp(eps->tps, COAPS_TCP_STR))
                     {
-                        strncpy(endpoint->addr, eps->addr, sizeof(endpoint->addr));
+                        strncpy(endpoint->addr, eps->addr, sizeof(endpoint->addr) - 1);
                         endpoint->port = eps->port;
                         endpoint->flags = (OCTransportFlags)(eps->family | OC_SECURE);
                         endpoint->adapter = OC_ADAPTER_TCP;
@@ -512,7 +511,7 @@ int parseClientResponse(OCClientResponse *clientResponse)
 #endif
                     if (!WithTcp && 0 == strcmp(eps->tps, COAPS_STR))
                     {
-                        strncpy(endpoint->addr, eps->addr, sizeof(endpoint->addr));
+                        strncpy(endpoint->addr, eps->addr, sizeof(endpoint->addr) - 1);
                         endpoint->port = eps->port;
                         endpoint->flags = (OCTransportFlags)(eps->family | OC_SECURE);
                         endpoint->adapter = OC_ADAPTER_IP;
index 4af2fef..c12f0bb 100644 (file)
@@ -194,7 +194,8 @@ int InitProxyRequest(void)
     memset(&option, 0, sizeof(option));
     option.protocolID = OC_COAP_ID;
     option.optionID = OC_RSRVD_PROXY_OPTION_ID;
-    strncpy((char*)option.optionData, httpResource, sizeof(option.optionData));
+    strncpy((char*)option.optionData, httpResource, sizeof(option.optionData) - 1);
+
     size_t opLen = strlen(httpResource);
     option.optionLength = opLen < sizeof(option.optionData) ? opLen : sizeof(option.optionData);
 
@@ -291,7 +292,7 @@ OCStackApplicationResult discoveryReqCB(void* ctx, OCDoHandle handle,
             {
                 if (0 == strcmp(eps->tps, "coaps"))
                 {
-                    strncpy(serverAddr.addr, eps->addr, sizeof(serverAddr.addr));
+                    strncpy(serverAddr.addr, eps->addr, sizeof(serverAddr.addr) - 1);
                     serverAddr.port = eps->port;
                     serverAddr.flags = (OCTransportFlags)(eps->family | OC_SECURE);
                     serverAddr.adapter = OC_ADAPTER_IP;
index 3439271..07eb03d 100644 (file)
@@ -55,10 +55,10 @@ void NSSetConsumerId(char * cId)
     NS_VERIFY_NOT_NULL_V(cId);
     char ** consumerId = NSGetConsumerId();
     NSOICFree(*consumerId);
-    *consumerId = (char *)OICMalloc(sizeof(char) * NS_DEVICE_ID_LENGTH);
+    *consumerId = (char *)OICMalloc(NS_DEVICE_ID_LENGTH);
     NS_VERIFY_NOT_NULL_V(*consumerId);
 
-    OICStrcpy(*consumerId, sizeof(char) * NS_DEVICE_ID_LENGTH, cId);
+    OICStrcpy(*consumerId, NS_DEVICE_ID_LENGTH, cId);
 }
 
 char * NSMakeRequestUriWithConsumerId(const char * uri)
@@ -276,7 +276,7 @@ static NSMessage * NSCreateMessage_internal(uint64_t id, const char * providerId
     NS_VERIFY_NOT_NULL(retMsg, NULL);
 
     retMsg->messageId = id;
-    OICStrcpy(retMsg->providerId, sizeof(char) * NS_DEVICE_ID_LENGTH, providerId);
+    OICStrcpy(retMsg->providerId, NS_DEVICE_ID_LENGTH, providerId);
     retMsg->title = NULL;
     retMsg->contentText = NULL;
     retMsg->sourceName = NULL;
@@ -543,7 +543,7 @@ NSProvider_internal * NSGetProvider(OCClientResponse * clientResponse)
     NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(newProvider, NULL,
           NSGetProviderPostClean(providerId, messageUri, syncUri, topicUri, connection));
 
-    OICStrcpy(newProvider->providerId, sizeof(char) * NS_DEVICE_ID_LENGTH, providerId);
+    OICStrcpy(newProvider->providerId, NS_DEVICE_ID_LENGTH, providerId);
     NSOICFree(providerId);
     newProvider->messageUri = messageUri;
     newProvider->syncUri = syncUri;
index e2e6a0f..a82870a 100644 (file)
@@ -260,7 +260,7 @@ NSSyncInfo * NSCreateSyncInfo_consumer(uint64_t msgId, const char * providerId,
 
     retSync->messageId = msgId;
     retSync->state = state;
-    OICStrcpy(retSync->providerId, sizeof(char) * NS_DEVICE_ID_LENGTH, providerId);
+    OICStrcpy(retSync->providerId, NS_DEVICE_ID_LENGTH, providerId);
 
     return retSync;
 }
index 54d3100..996683e 100644 (file)
@@ -28,6 +28,7 @@
 #include "OCPlatform.h"
 #include "octypes.h"
 #include "ocstack.h"
+#include "oic_string.h"
 #include "ocpayload.h"
 #include "cainterface.h"
 
@@ -155,11 +156,11 @@ namespace
         }
         addr->adapter = (ninfo[0].adapter == CA_ALL_ADAPTERS) ? OC_ADAPTER_IP :
                 (OCTransportAdapter) ninfo[0].adapter;
-        strncpy(addr->addr, ninfo[0].addr, sizeof(ninfo[0].addr));
+        OICStrcpy(addr->addr, sizeof(ninfo[0].addr), ninfo[0].addr);
         addr->flags = (OCTransportFlags)ninfo[0].flags;
         addr->ifindex = ninfo[0].ifindex;
         addr->port = ninfo[0].port;
-        strncpy(addr->remoteId, ninfo[0].remoteId, 37);
+        OICStrcpy(addr->remoteId, NS_UUID_STRING_SIZE, ninfo[0].remoteId);
 
         free(ninfo);
         ninfo = NULL;
@@ -261,11 +262,11 @@ namespace
         testResponse->addr = testAddr;
         testResponse->devAddr = *testAddr;
         testResponse->connType = CT_ADAPTER_IP;
-        testResponse->identity.id_length = 37;
-        strncpy((char *)(testResponse->identity.id), testProviderID.c_str(), 37);
+        testResponse->identity.id_length = NS_UUID_STRING_SIZE;
+        OICStrcpy((char *)(testResponse->identity.id), NS_UUID_STRING_SIZE, testProviderID.c_str());
         testResponse->numRcvdVendorSpecificHeaderOptions = 0;
         testResponse->resourceUri = (char*)malloc(sizeof(char)*notiUri.size() + 1);
-        strncpy((char*)testResponse->resourceUri, notiUri.c_str(), notiUri.size()+1);
+        OICStrcpy((char*)testResponse->resourceUri, notiUri.size()+1, notiUri.c_str());
         testResponse->result = OC_STACK_OK;
         testResponse->sequenceNumber = 1;
         testResponse->payload = NULL;
@@ -304,7 +305,7 @@ namespace
         }
         provider->accessPolicy = NSSelector::NS_SELECTION_CONSUMER;
         provider->state = NS_DISCOVERED;
-        strncpy(provider->providerId, testProviderID.c_str(), NS_UUID_STRING_SIZE);
+        OICStrcpy(provider->providerId, NS_UUID_STRING_SIZE, testProviderID.c_str());
         provider->messageUri = strdup("/notificationTest/message");
         provider->syncUri = strdup("/notificationTest/sync");
         provider->topicUri = strdup("/notificationTest/topic");
index 87c205a..991d2c5 100644 (file)
@@ -428,7 +428,7 @@ TEST(NotificationProviderTest, ExpectSuccessSendMessage)
 TEST(NotificationProviderTest, ExpectCopyConsumer)
 {
     auto consumer = (NSConsumer *)malloc(sizeof(NSConsumer));
-    strncpy(consumer->consumerId, g_consumerID, (size_t)NS_UUID_STRING_SIZE);
+    OICStrcpy(consumer->consumerId, NS_UUID_STRING_SIZE, g_consumerID);
 
     auto copied = NSDuplicateConsumer(consumer);
     EXPECT_EQ(0, strcmp(copied->consumerId, consumer->consumerId));