Fixed string-handling bugs for |csdk_security| module
authorKyungsun Cho <goodsun.cho@samsung.com>
Wed, 6 Apr 2016 08:33:02 +0000 (17:33 +0900)
committerRandeep Singh <randeep.s@samsung.com>
Wed, 6 Apr 2016 11:43:15 +0000 (11:43 +0000)
this change is for fixing string-handling bugs on |csdk_security|.
|OICSstr-xxx| APIs have |destSize| as its second parameter for
preventing buffer-overflow, but it makes the application crash,
mis-inputting |srcSize| as its second.

[patch #1] initial commit
[patch #2] fixed bugs related to get destSize

Change-Id: I4874b17fa71c13741226cb8650cfa4d53741b182
Signed-off-by: Kyungsun Cho <goodsun.cho@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/7635
Reviewed-by: Randeep Singh <randeep.s@samsung.com>
Tested-by: Randeep Singh <randeep.s@samsung.com>
resource/csdk/security/provisioning/src/pmutility.c
resource/csdk/security/src/verresource.c
resource/csdk/security/unittest/aclresourcetest.cpp
resource/csdk/security/unittest/credentialresource.cpp

index 54f7a3c..e55e9a6 100644 (file)
@@ -239,7 +239,7 @@ OCStackResult UpdateSecVersionOfDevice(OCProvisionDev_t **ppDevicesList, const c
         return OC_STACK_ERROR;
     }
 
-    OICStrcpy(ptr->secVer, strlen(secVer), secVer);
+    OICStrcpy(ptr->secVer, MAX_VERSION_LEN, secVer);
 
     return OC_STACK_OK;
 }
@@ -308,7 +308,7 @@ OCProvisionDev_t* PMCloneOCProvisionDev(const OCProvisionDev_t* src)
     }
     else
     {
-        OICStrcpy(newDev->secVer, strlen(src->secVer), src->secVer);
+        OICStrcpy(newDev->secVer, MAX_VERSION_LEN, src->secVer);
     }
 
     newDev->securePort = src->securePort;
index 7d60bb3..0bd3f75 100644 (file)
@@ -315,7 +315,7 @@ OCStackResult InitVerResource()
 {
     OCStackResult ret = OC_STACK_ERROR;
 
-    OICStrcpy(gVer.secv, strlen(SECURITY_VERSION)+1, SECURITY_VERSION);
+    OICStrcpy(gVer.secv, MAX_VERSION_LEN, SECURITY_VERSION);
 
     //Read device id from doxm
     OicUuid_t deviceID = {.id={0}};
index e759256..558406a 100644 (file)
@@ -315,12 +315,12 @@ static OCStackResult  populateAcl(OicSecAcl_t *acl,  int numRsrc)
     VERIFY_NON_NULL(TAG, acl->resources, ERROR);
     acl->resources[0] = (char*)OICMalloc(strlen("/a/led")+1);
     VERIFY_NON_NULL(TAG, acl->resources[0], ERROR);
-    OICStrcpy(acl->resources[0], sizeof(acl->resources[0]) - 1, "/a/led");
+    OICStrcpy(acl->resources[0], strlen("/a/led")+1, "/a/led");
     if(numRsrc == 2)
     {
         acl->resources[1] = (char*)OICMalloc(strlen("/a/fan")+1);
         VERIFY_NON_NULL(TAG, acl->resources[1], ERROR);
-        OICStrcpy(acl->resources[1], sizeof(acl->resources[1]) - 1, "/a/fan");
+        OICStrcpy(acl->resources[1], strlen("/a/fan")+1, "/a/fan");
     }
     acl->permission = 6;
     memcpy(acl->rownerID.id, "1111111111111111", sizeof(acl->rownerID.id));
index 894ff19..fd63aa1 100644 (file)
@@ -40,7 +40,8 @@ OicSecCred_t * getCredList()
     OicSecCred_t *cred = (OicSecCred_t *)OICCalloc(1, sizeof(*cred));
     VERIFY_NON_NULL(TAG, cred, ERROR);
     cred->credId = 1234;
-    OICStrcpy((char *)cred->subject.id, sizeof(cred->subject.id)+1, "1111111111111111");
+    // use |memcpy| for copying full-lengthed UUID without null termination
+    memcpy(cred->subject.id, "1111111111111111", sizeof(cred->subject.id));
 
 #if 0
     cred->roleIdsLen = 2;
@@ -48,18 +49,19 @@ OicSecCred_t * getCredList()
     VERIFY_NON_NULL(TAG, cred->roleIds, ERROR);
     OICStrcpy((char *)cred->roleIds[0].id, sizeof(cred->roleIds[0].id), "role11");
     OICStrcpy((char *)cred->roleIds[1].id, sizeof(cred->roleIds[1].id), "role12");
-
 #endif
 
     cred->credType = SYMMETRIC_PAIR_WISE_KEY;
     cred->privateData.data = (uint8_t *)OICCalloc(1, strlen("My private Key11") + 1);
     VERIFY_NON_NULL(TAG, cred->privateData.data, ERROR);
     OICStrcpy((char *)cred->privateData.data, strlen("My private Key11")+1,"My private Key11");
-    OICStrcpy((char *)cred->rownerID.id, sizeof(cred->rownerID.id), "aaaaaaaaaaaaaaaa");
+    // use |memcpy| for copying full-lengthed UUID without null termination
+    memcpy(cred->rownerID.id, "aaaaaaaaaaaaaaaa", sizeof(cred->rownerID.id));
     cred->next = (OicSecCred_t*)OICCalloc(1, sizeof(*cred->next));
     VERIFY_NON_NULL(TAG, cred->next, ERROR);
     cred->next->credId = 5678;
-    OICStrcpy((char *)cred->next->subject.id, sizeof(cred->next->subject.id)+1, "2222222222222222");
+    // use |memcpy| for copying full-lengthed UUID without null termination
+    memcpy(cred->next->subject.id, "2222222222222222", sizeof(cred->next->subject.id));
 #if 0
     cred->next->roleIdsLen = 0;
 #endif
@@ -69,12 +71,13 @@ OicSecCred_t * getCredList()
     VERIFY_NON_NULL(TAG, cred->next->privateData.data, ERROR);
     OICStrcpy((char *)cred->next->privateData.data, sz, "My private Key21");
 #if 0
-    sz = strlen("My Public Key123") + 1
+    sz = strlen("My Public Key123") + 1;
     cred->next->publicData.data = (char *)OICCalloc(1, sz);
     VERIFY_NON_NULL(TAG, cred->next->publicData.data, ERROR);
     OICStrcpy(cred->next->publicData.data, sz,"My Public Key123");
 #endif
-    OICStrcpy((char *)cred->next->rownerID.id, sizeof(cred->next->rownerID.id), "bbbbbbbbbbbbbbbb");
+    // use |memcpy| for copying full-lengthed UUID without null termination
+    memcpy(cred->next->rownerID.id, "bbbbbbbbbbbbbbbb", sizeof(cred->next->rownerID.id));
 
     return cred;
 
@@ -291,10 +294,10 @@ TEST(CredResourceTest, GetCredResourceDataNULLSubject)
 TEST(CredResourceTest, GenerateCredentialValidInput)
 {
     OicUuid_t rownerID = {{0}};
-    OICStrcpy((char *)rownerID.id, strlen("ownersId21"), "ownersId21");
+    OICStrcpy((char *)rownerID.id, sizeof(rownerID.id), "ownersId21");
 
     OicUuid_t subject = {{0}};
-    OICStrcpy((char *)subject.id, strlen("subject11"), "subject11");
+    OICStrcpy((char *)subject.id, sizeof(subject.id), "subject11");
 
     uint8_t privateKey[] = "My private Key11";
     OicSecKey_t key = {privateKey, sizeof(privateKey)};