Fixed bug IOT-791

Fixed seg fault caused by error handling conditions in function
AmsMgrAclReqCallback while trying to access uninitialized
amsMgrContext field in PEContext.

Change-Id: I65d3d76fc476f848a82e606f5f7f2a2bb3540935
Signed-off-by: Shilpa Sodani <shilpa.a.sodani@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/3511
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Sachin Agrawal <sachin.agrawal@intel.com>

diff --git a/resource/csdk/security/include/internal/amsmgr.h b/resource/csdk/security/include/internal/amsmgr.h
index c83530f..d9d74b5 100644 (file)
--- a/resource/csdk/security/include/internal/amsmgr.h
+++ b/resource/csdk/security/include/internal/amsmgr.h
@@ -125,11 +125,19 @@ bool FoundAmaclForRequest(PEContext_t *context);
 
 /*
  * This method is used by Policy engine to process AMS request
- * *
+ *
  * @param context   Policy engine context.
  *
- * @return          None
  */
 void ProcessAMSRequest(PEContext_t *context);
 
+
+/*
+ * This method is used by Policy engine to free AMS context requestInfo
+ *
+ * @param requestInfo   pointer to CARequestInfo_t.
+ *
+ */
+void FreeCARequestInfo(CARequestInfo_t *requestInfo);
+
 #endif //IOTVT_SRM_AMSMGR_H

diff --git a/resource/csdk/security/src/amsmgr.c b/resource/csdk/security/src/amsmgr.c
index c0f7377..8779b0b 100644 (file)
--- a/resource/csdk/security/src/amsmgr.c
+++ b/resource/csdk/security/src/amsmgr.c
@@ -181,13 +181,14 @@ static OCStackApplicationResult SecurePortDiscoveryCallback(void *ctx, OCDoHandl
         !clientResponse->payload||
         (PAYLOAD_TYPE_DISCOVERY != clientResponse->payload->type)||
         (OC_STACK_OK != clientResponse->result))
-        {
-            OC_LOG_V(ERROR, TAG, "%s Invalid Response ", __func__);
-            SRMSendResponse(ACCESS_DENIED_AMS_SERVICE_ERROR);
-            return OC_STACK_DELETE_TRANSACTION;
-        }
+    {
+        OC_LOG_V(ERROR, TAG, "%s Invalid Response ", __func__);
+        SRMSendResponse(ACCESS_DENIED_AMS_SERVICE_ERROR);
+        return OC_STACK_DELETE_TRANSACTION;
+    }
 
     PEContext_t *context = (PEContext_t *) ctx;
+
     (void)handle;
     if (context->state != AWAITING_AMS_RESPONSE)
     {
@@ -196,6 +197,7 @@ static OCStackApplicationResult SecurePortDiscoveryCallback(void *ctx, OCDoHandl
         SRMSendResponse(context->retVal);
         return OC_STACK_DELETE_TRANSACTION;
     }
+
     OCResourcePayload* resPayload = ((OCDiscoveryPayload*)clientResponse->payload)->resources;
 
     //Verifying if the ID of the sender is an AMS service that this device trusts.
@@ -203,6 +205,7 @@ static OCStackApplicationResult SecurePortDiscoveryCallback(void *ctx, OCDoHandl
        memcmp(context->amsMgrContext->amsDeviceId.id, resPayload->sid,
                     sizeof(context->amsMgrContext->amsDeviceId.id)) != 0)
     {
+        OC_LOG_V(ERROR, TAG, "%s Invalid AMS device", __func__);
         context->retVal = ACCESS_DENIED_AMS_SERVICE_ERROR;
         SRMSendResponse(context->retVal);
         return OC_STACK_DELETE_TRANSACTION;
@@ -217,6 +220,7 @@ static OCStackApplicationResult SecurePortDiscoveryCallback(void *ctx, OCDoHandl
         }
     }
     OC_LOG(INFO, TAG, "Can not find secure port information");
+
     context->retVal = ACCESS_DENIED_AMS_SERVICE_ERROR;
     SRMSendResponse(context->retVal);
     return OC_STACK_DELETE_TRANSACTION;
@@ -279,8 +283,9 @@ static OCStackApplicationResult AmsMgrAclReqCallback(void *ctx, OCDoHandle handl
         (PAYLOAD_TYPE_SECURITY != clientResponse->payload->type) ||
         (clientResponse->result != OC_STACK_OK))
     {
+        OC_LOG_V(ERROR, TAG, "%s Invalid Response ", __func__);
         SRMSendResponse(ACCESS_DENIED_AMS_SERVICE_ERROR);
-        goto exit;
+        return OC_STACK_DELETE_TRANSACTION;
     }
 
     if (context->state != AWAITING_AMS_RESPONSE)
@@ -315,8 +320,6 @@ static OCStackApplicationResult AmsMgrAclReqCallback(void *ctx, OCDoHandle handl
 exit:
     context->retVal = ACCESS_DENIED_AMS_SERVICE_ERROR;
     SRMSendResponse(context->retVal);
-    FreeCARequestInfo(context->amsMgrContext->requestInfo);
-    OICFree(context->amsMgrContext->endpoint);
     return OC_STACK_DELETE_TRANSACTION;
 }
 
@@ -351,6 +354,11 @@ exit:
 
 void FreeCARequestInfo(CARequestInfo_t *requestInfo)
 {
+    if(NULL == requestInfo)
+    {
+        OC_LOG_V(ERROR, TAG, "%s: Can't free memory. Received NULL requestInfo", __func__);
+        return;
+    }
     OICFree(requestInfo->info.token);
     OICFree(requestInfo->info.options);
     OICFree(requestInfo->info.payload);
@@ -404,6 +412,7 @@ void ProcessAMSRequest(PEContext_t *context)
                 if(OC_STACK_OK == DiscoverAmsService(context))
                 {
                     context->retVal = ACCESS_WAITING_FOR_AMS;
+                    context->state = AWAITING_AMS_RESPONSE;
                 }
                 else
                 {

diff --git a/resource/csdk/security/src/policyengine.c b/resource/csdk/security/src/policyengine.c
index ef7b005..36b03fa 100644 (file)
--- a/resource/csdk/security/src/policyengine.c
+++ b/resource/csdk/security/src/policyengine.c
@@ -96,6 +96,9 @@ void SetPolicyEngineState(PEContext_t *context, const PEState_t state)
     context->matchingAclFound = false;
     context->amsProcessing = false;
     context->retVal = ACCESS_DENIED_POLICY_ENGINE_ERROR;
+
+    FreeCARequestInfo(context->amsMgrContext->requestInfo);
+    OICFree(context->amsMgrContext->endpoint);
     memset(context->amsMgrContext, 0, sizeof(AmsMgrContext_t));
 
     // Set state.
@@ -442,14 +445,7 @@ SRMAccessResponse_t CheckPermission(
     // Capture retVal before resetting state for next request.
     retVal = context->retVal;
 
-    //Change the state of PE to "AWAITING_AMS_RESPONSE", if waiting
-    //for response from AMS service else to "AWAITING_REQUEST"
-    if(ACCESS_WAITING_FOR_AMS == retVal)
-    {
-        OC_LOG(INFO, TAG, "Setting PE State to AWAITING_AMS_RESPONSE");
-        context->state = AWAITING_AMS_RESPONSE;
-    }
-    else if(!context->amsProcessing)
+   if(!context->amsProcessing)
     {
         OC_LOG(INFO, TAG, "Resetting PE context and PE State to AWAITING_REQUEST");
         SetPolicyEngineState(context, AWAITING_REQUEST);
@@ -466,14 +462,18 @@ exit:
  */
 OCStackResult InitPolicyEngine(PEContext_t *context)
 {
-    if(NULL== context)
+    if(NULL == context)
     {
         return OC_STACK_ERROR;
     }
 
-    context->amsMgrContext = (AmsMgrContext_t *)OICMalloc(sizeof(AmsMgrContext_t));
-    SetPolicyEngineState(context, AWAITING_REQUEST);
+    context->amsMgrContext = (AmsMgrContext_t *)OICCalloc(1, sizeof(AmsMgrContext_t));
+    if(NULL == context->amsMgrContext)
+    {
+        return OC_STACK_ERROR;
+    }
 
+    SetPolicyEngineState(context, AWAITING_REQUEST);
     return OC_STACK_OK;
 }
 

diff --git a/resource/csdk/security/src/secureresourcemanager.c b/resource/csdk/security/src/secureresourcemanager.c
index 6d55727..a91bc53 100644 (file)
--- a/resource/csdk/security/src/secureresourcemanager.c
+++ b/resource/csdk/security/src/secureresourcemanager.c
@@ -69,17 +69,26 @@ void SRMRegisterProvisioningResponseHandler(SPResponseCallback respHandler)
 static void SRMSendUnAuthorizedAccessresponse(PEContext_t *context)
 {
     CAResponseInfo_t responseInfo = {.result = CA_EMPTY};
+
+    if(NULL == context ||
+       NULL == context->amsMgrContext->requestInfo)
+    {
+        OC_LOG_V(ERROR, TAG, "%s : NULL Parameter(s)",__func__);
+        return;
+    }
+
     memcpy(&responseInfo.info, &(context->amsMgrContext->requestInfo->info),
             sizeof(responseInfo.info));
     responseInfo.info.payload = NULL;
     responseInfo.result = CA_UNAUTHORIZED_REQ;
-    if (CA_STATUS_OK != CASendResponse(context->amsMgrContext->endpoint, &responseInfo))
+
+    if (CA_STATUS_OK == CASendResponse(context->amsMgrContext->endpoint, &responseInfo))
     {
-        OC_LOG(ERROR, TAG, "Failed in sending response to a unauthorized request!");
+        OC_LOG(DEBUG, TAG, "Succeed in sending response to a unauthorized request!");
     }
     else
     {
-        OC_LOG(INFO, TAG, "Succeed in sending response to a unauthorized request!");
+        OC_LOG(ERROR, TAG, "Failed in sending response to a unauthorized request!");
     }
 }
 
@@ -92,7 +101,7 @@ void SRMSendResponse(SRMAccessResponse_t responseVal)
     {
         OC_LOG_V(INFO, TAG, "%s : Access granted. Passing Request to RI layer", __func__);
         if (!g_policyEngineContext.amsMgrContext->endpoint ||
-                !g_policyEngineContext.amsMgrContext->requestInfo)
+            !g_policyEngineContext.amsMgrContext->requestInfo)
         {
             OC_LOG_V(ERROR, TAG, "%s : Invalid arguments", __func__);
             SRMSendUnAuthorizedAccessresponse(&g_policyEngineContext);
@@ -108,7 +117,7 @@ void SRMSendResponse(SRMAccessResponse_t responseVal)
     }
 
 exit:
-    //Resting PE state to AWAITING_REQUEST
+    //Resetting PE state to AWAITING_REQUEST
     SetPolicyEngineState(&g_policyEngineContext, AWAITING_REQUEST);
 }