From 0c88b91ed0cda615b6ec8b7d6eb6548607ab6cc8 Mon Sep 17 00:00:00 2001 From: Shilpa Sodani Date: Mon, 5 Oct 2015 13:09:13 -0700 Subject: [PATCH] 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/3511 Tested-by: jenkins-iotivity Reviewed-by: Sachin Agrawal --- resource/csdk/security/include/internal/amsmgr.h | 12 +++++++++-- resource/csdk/security/src/amsmgr.c | 25 +++++++++++++++------- resource/csdk/security/src/policyengine.c | 22 +++++++++---------- resource/csdk/security/src/secureresourcemanager.c | 19 +++++++++++----- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/resource/csdk/security/include/internal/amsmgr.h b/resource/csdk/security/include/internal/amsmgr.h index c83530f..d9d74b5 100644 --- 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 --- 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 --- 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 --- 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); } -- 2.7.4