security: Fix to access dereferenced object memory in OCSecure class

[Joonghwan Lee]

This patch fix below invalid memory access in OCSecure class
- invalid DisplayNumContext object access
- invalid UserConfirmNumContext object access
- minor fix : prevent duplicated call of SetResult()

[Philippe Coval]

Ported from 1.2-rel branch to master

  Conflicts:
	resource/csdk/security/provisioning/src/ownershiptransfermanager.c

Change-Id: I623c856f7086cf7fe7333e69f682070434961fbe
Signed-off-by: Joonghwan Lee <jh05.lee@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/16369
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Chul Lee <chuls.lee@samsung.com>
Reviewed-by: Randeep Singh <randeep.s@samsung.com>
Author: Joonghwan Lee <jh05.lee@samsung.com>
Signed-off-by: Philippe Coval <philippe.coval@osg.samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/16949

diff --git a/resource/csdk/security/include/oxmverifycommon.h b/resource/csdk/security/include/oxmverifycommon.h
index a90eaef..bc1b2df 100644 (file)
--- a/resource/csdk/security/include/oxmverifycommon.h
+++ b/resource/csdk/security/include/oxmverifycommon.h
@@ -80,7 +80,7 @@ void SetDisplayNumCB(void * ptr, DisplayNumCallback displayNumCB);
 /*
  * Unset Callback for displaying verification PIN
  */
-void UnsetDisplayNumCB();
+void* UnsetDisplayNumCB();
 
 /*
  * Set Callback for getting user confirmation
@@ -90,7 +90,7 @@ void SetUserConfirmCB(void * ptr, UserConfirmCallback userConfirmCB);
 /*
  * Unset Callback for getting user confirmation
  */
-void UnsetUserConfirmCB();
+void* UnsetUserConfirmCB();
 
 /*
  * Set verification method option.

diff --git a/resource/csdk/security/provisioning/src/ownershiptransfermanager.c b/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
index 3b841ec..6d73f8f 100644 (file)
--- a/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
+++ b/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
@@ -984,6 +984,9 @@ static OCStackApplicationResult OwnerUuidUpdateHandler(void *ctx, OCDoHandle UNU
                     {
                         OIC_LOG(WARNING, TAG, "OwnerUuidUpdateHandler : SRPResetDevice error");
                     }
+                    OIC_LOG(ERROR, TAG, "OwnerUuidUpdateHandler:Failed to verify user confirm");
+                    SetResult(otmCtx, res);
+                    return OC_STACK_DELETE_TRANSACTION;
                 }
             }
 

diff --git a/resource/csdk/security/src/oxmverifycommon.c b/resource/csdk/security/src/oxmverifycommon.c
index 3f13ed7..169fb65 100644 (file)
--- a/resource/csdk/security/src/oxmverifycommon.c
+++ b/resource/csdk/security/src/oxmverifycommon.c
@@ -44,9 +44,12 @@ void SetDisplayNumCB(void * ptr, DisplayNumCallback displayNumCB)
     gDisplayNumContext.context = ptr;
 }
 
-void UnsetDisplayNumCB()
+void* UnsetDisplayNumCB()
 {
+    void *prevctx = gDisplayNumContext.context;
     gDisplayNumContext.callback = NULL;
+    gDisplayNumContext.context= NULL;
+    return prevctx;
 }
 
 void SetUserConfirmCB(void * ptr, UserConfirmCallback userConfirmCB)
@@ -60,9 +63,12 @@ void SetUserConfirmCB(void * ptr, UserConfirmCallback userConfirmCB)
     gUserConfirmContext.context = ptr;
 }
 
-void UnsetUserConfirmCB()
+void* UnsetUserConfirmCB()
 {
+    void *prevctx = gUserConfirmContext.context;
     gUserConfirmContext.callback = NULL;
+    gUserConfirmContext.context = NULL;
+    return prevctx;
 }
 
 void SetVerifyOption(VerifyOptionBitmask_t verifyOption)

diff --git a/resource/provisioning/src/OCProvisioningManager.cpp b/resource/provisioning/src/OCProvisioningManager.cpp
index 1e08a3c..5590065 100644 (file)
--- a/resource/provisioning/src/OCProvisioningManager.cpp
+++ b/resource/provisioning/src/OCProvisioningManager.cpp
@@ -736,14 +736,21 @@ namespace OC
     OCStackResult OCSecure::displayNumCallbackWrapper(void* ctx,
             uint8_t verifNum[MUTUAL_VERIF_NUM_LEN])
     {
-        OCStackResult result;
+        uint8_t *number = NULL;
 
         DisplayNumContext* context = static_cast<DisplayNumContext*>(ctx);
-        uint8_t *number = new uint8_t[MUTUAL_VERIF_NUM_LEN];
-        memcpy(number, verifNum, MUTUAL_VERIF_NUM_LEN);
-        result = context->callback(number);
-        delete context;
-        return result;
+        if (!context)
+        {
+            oclog() << "Invalid context";
+            return OC_STACK_INVALID_PARAM;
+        }
+
+        if (NULL != verifNum) {
+            number = new uint8_t[MUTUAL_VERIF_NUM_LEN];
+            memcpy(number, verifNum, MUTUAL_VERIF_NUM_LEN);
+        }
+
+        return context->callback(number);
     }
 
     OCStackResult OCSecure::registerDisplayNumCallback(DisplayNumCB displayNumCB)
@@ -754,9 +761,14 @@ namespace OC
             return OC_STACK_INVALID_CALLBACK;
         }
 
-        OCStackResult result;
-        auto cLock = OCPlatform_impl::Instance().csdkLock().lock();
+        OCStackResult result = OCSecure::deregisterDisplayNumCallback();
+        if (OC_STACK_OK != result)
+        {
+            oclog() << "Failed to de-register callback for display."<<std::endl;
+            return result;
+        }
 
+        auto cLock = OCPlatform_impl::Instance().csdkLock().lock();
         if (cLock)
         {
             DisplayNumContext* context = new DisplayNumContext(displayNumCB);
@@ -780,7 +792,12 @@ namespace OC
         if (cLock)
         {
             std::lock_guard<std::recursive_mutex> lock(*cLock);
-            UnsetDisplayNumCB();
+            DisplayNumContext* context = static_cast<DisplayNumContext*>(UnsetDisplayNumCB());
+            if (context)
+            {
+                oclog() << "Delete registered display num context"<<std::endl;
+                delete context;
+            }
             result = OC_STACK_OK;
         }
         else
@@ -793,12 +810,14 @@ namespace OC
 
     OCStackResult OCSecure::confirmUserCallbackWrapper(void* ctx)
     {
-        OCStackResult result;
-
         UserConfirmNumContext* context = static_cast<UserConfirmNumContext*>(ctx);
-        result = context->callback();
-        delete context;
-        return result;
+        if (!context)
+        {
+            oclog() << "Invalid context";
+            return OC_STACK_INVALID_PARAM;
+        }
+
+        return context->callback();
     }
 
     OCStackResult OCSecure::registerUserConfirmCallback(UserConfirmNumCB userConfirmCB)
@@ -809,7 +828,13 @@ namespace OC
             return OC_STACK_INVALID_CALLBACK;
         }
 
-        OCStackResult result;
+        OCStackResult result = OCSecure::deregisterUserConfirmCallback();
+        if (OC_STACK_OK != result)
+        {
+            oclog() << "Failed to de-register callback for comfirm."<<std::endl;
+            return result;
+        }
+
         auto cLock = OCPlatform_impl::Instance().csdkLock().lock();
         if (cLock)
         {
@@ -834,7 +859,12 @@ namespace OC
         if (cLock)
         {
             std::lock_guard<std::recursive_mutex> lock(*cLock);
-            UnsetUserConfirmCB();
+            UserConfirmNumContext* context = static_cast<UserConfirmNumContext*>(UnsetUserConfirmCB());
+            if (context)
+            {
+                oclog() << "Delete registered user confirm context"<<std::endl;
+                delete context;
+            }
             result = OC_STACK_OK;
         }
         else