From 6410f7a2a9d95eda15c643c248e1af0970da4bf5 Mon Sep 17 00:00:00 2001 From: samanway Date: Wed, 5 Feb 2020 16:44:29 +0530 Subject: [CONPRO-1560] iotivity crash in data receiver thread - In function CADropSecondMessage(), an input argument pointer NULL check was missing, added the same - In function CAGetInfoFromPDU(), 512 bytes memory was getting assingned from stack, which may fail in case stack overflow. Hence allocated memory from heap. this fix is already in IoTivity 2.0-rel. https://github.sec.samsung.net/RS7-IOTIVITY/IoTivity/pull/663 (cherry-picked from 7c12ddb7711cbb9af9d7c68635c171692222bdb4) Change-Id: I09e987240e7d37460ba7c567f7b6593d6b3b9e3a Signed-off-by: samanway-dey Signed-off-by: DoHyun Pyun --- resource/csdk/connectivity/src/camessagehandler.c | 5 ++- resource/csdk/connectivity/src/caprotocolmessage.c | 40 +++++++++++++++++----- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/resource/csdk/connectivity/src/camessagehandler.c b/resource/csdk/connectivity/src/camessagehandler.c index ef3e3e792..da4320d30 100644 --- a/resource/csdk/connectivity/src/camessagehandler.c +++ b/resource/csdk/connectivity/src/camessagehandler.c @@ -748,7 +748,10 @@ static bool CADropSecondMessage(CAHistory_t *history, const CAEndpoint_t *ep, ui { return false; } - + if (!history) + { + return false; + } if (tokenLength > CA_MAX_TOKEN_LEN) { /* diff --git a/resource/csdk/connectivity/src/caprotocolmessage.c b/resource/csdk/connectivity/src/caprotocolmessage.c index 0ab1ae7f3..e23fbcf30 100755 --- a/resource/csdk/connectivity/src/caprotocolmessage.c +++ b/resource/csdk/connectivity/src/caprotocolmessage.c @@ -763,7 +763,11 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, } coap_opt_t *option = NULL; - char optionResult[CA_MAX_URI_LENGTH] = {0}; + char *optionResult = (char *)OICCalloc(1, CA_MAX_URI_LENGTH * sizeof(char)); + if (NULL == optionResult) + { + goto exit; + } uint32_t idx = 0; uint32_t optionLength = 0; bool isfirstsetflag = false; @@ -774,10 +778,15 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, while ((option = coap_option_next(&opt_iter))) { - char buf[COAP_MAX_PDU_SIZE] = {0}; + char *buf = (char *)OICCalloc(1, COAP_MAX_PDU_SIZE * sizeof(char)); + if (NULL == buf) + { + goto exit; + } + uint32_t bufLength = CAGetOptionData(opt_iter.type, (uint8_t *)(COAP_OPT_VALUE(option)), - COAP_OPT_LENGTH(option), (uint8_t *)buf, sizeof(buf)); + COAP_OPT_LENGTH(option), (uint8_t *)buf, COAP_MAX_PDU_SIZE); if (bufLength) { OIC_LOG_V(DEBUG, TAG, "COAP URI element : %s", buf); @@ -789,13 +798,14 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, optionResult[optionLength] = '/'; optionLength++; // Make sure there is enough room in the optionResult buffer - if ((optionLength + bufLength) < sizeof(optionResult)) + if ((optionLength + bufLength) < CA_MAX_URI_LENGTH) { memcpy(&optionResult[optionLength], buf, bufLength); optionLength += bufLength; } else { + OICFree(buf); goto exit; } } @@ -804,13 +814,14 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, if (COAP_OPTION_URI_PATH == opt_iter.type) { // Make sure there is enough room in the optionResult buffer - if (optionLength < sizeof(optionResult)) + if (optionLength < CA_MAX_URI_LENGTH) { optionResult[optionLength] = '/'; optionLength++; } else { + OICFree(buf); goto exit; } } @@ -819,7 +830,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, if (false == isQueryBeingProcessed) { // Make sure there is enough room in the optionResult buffer - if (optionLength < sizeof(optionResult)) + if (optionLength < CA_MAX_URI_LENGTH) { optionResult[optionLength] = '?'; optionLength++; @@ -827,31 +838,34 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, } else { + OICFree(buf); goto exit; } } else { // Make sure there is enough room in the optionResult buffer - if (optionLength < sizeof(optionResult)) + if (optionLength < CA_MAX_URI_LENGTH) { optionResult[optionLength] = ';'; optionLength++; } else { + OICFree(buf); goto exit; } } } // Make sure there is enough room in the optionResult buffer - if ((optionLength + bufLength) < sizeof(optionResult)) + if ((optionLength + bufLength) < CA_MAX_URI_LENGTH) { memcpy(&optionResult[optionLength], buf, bufLength); optionLength += bufLength; } else { + OICFree(buf); goto exit; } } @@ -917,6 +931,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, } } } + OICFree(buf); } unsigned char* token = NULL; @@ -932,6 +947,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, { OIC_LOG(ERROR, TAG, "Out of memory"); OICFree(outInfo->options); + OICFree(optionResult); return CA_MEMORY_ALLOC_FAILED; } memcpy(outInfo->token, token, token_length); @@ -951,14 +967,16 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, OIC_LOG(ERROR, TAG, "Out of memory"); OICFree(outInfo->options); OICFree(outInfo->token); + OICFree(optionResult); return CA_MEMORY_ALLOC_FAILED; } memcpy(outInfo->payload, pdu->data, dataSize); outInfo->payloadSize = dataSize; } - if (optionResult[0] != '\0') + if (optionResult[0] != '\0' && optionLength>=0 && optionLengthresourceUri = OICStrdup(optionResult); if (!outInfo->resourceUri) @@ -966,6 +984,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, OIC_LOG(ERROR, TAG, "Out of memory"); OICFree(outInfo->options); OICFree(outInfo->token); + OICFree(optionResult); return CA_MEMORY_ALLOC_FAILED; } } @@ -983,15 +1002,18 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint, OIC_LOG(ERROR, TAG, "Out of memory"); OICFree(outInfo->options); OICFree(outInfo->token); + OICFree(optionResult); return CA_MEMORY_ALLOC_FAILED; } } #endif + OICFree(optionResult); return CA_STATUS_OK; exit: OIC_LOG(ERROR, TAG, "buffer too small"); OICFree(outInfo->options); + OICFree(optionResult); return CA_STATUS_FAILED; } -- cgit v1.2.3