From 78e3f7669d281193b7a9d3b322368a5041f1ee44 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Thu, 18 Dec 2014 14:52:20 -0800 Subject: [PATCH] Fixed bad memory allocation, which was causing corruption Previously the stack was allocating based on a variable that was never actually set, and despite that was likely allocating based on the incorrect data. This patch ensures that the allocation is based on the actual size of the string. Change-Id: Ie2feafe665b870919e17222dade5548794b6ce67 Signed-off-by: Erich Keane --- resource/csdk/occoap/src/occoap.c | 4 ++-- .../csdk/stack/include/internal/ocserverrequest.h | 4 ++-- .../csdk/stack/include/internal/ocstackinternal.h | 2 +- resource/csdk/stack/src/ocserverrequest.c | 22 ++++++++++++++++------ resource/csdk/stack/src/ocstack.c | 6 ++++++ 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/resource/csdk/occoap/src/occoap.c b/resource/csdk/occoap/src/occoap.c index 38f8c0b..947c8ee 100644 --- a/resource/csdk/occoap/src/occoap.c +++ b/resource/csdk/occoap/src/occoap.c @@ -131,7 +131,7 @@ static void HandleCoAPRequests(struct coap_context_t *ctx, coap_block_t rcvdBlock2; memset(&rcvdBlock1, COAP_BLOCK_FILL_VALUE, sizeof(coap_block_t)); memset(&rcvdBlock2, COAP_BLOCK_FILL_VALUE, sizeof(coap_block_t)); - uint16_t rcvdSize1 = 0; + size_t rcvdSize1 = 0; coap_pdu_t * rcvdPdu = rcvdRequest->pdu; coap_pdu_t * sendPdu = NULL; coap_send_flags_t sendFlag; @@ -211,7 +211,7 @@ static void HandleCoAPRequests(struct coap_context_t *ctx, else { // No block1 received - rcvdSize1 = strlen((const char *)protocolRequest.reqJSONPayload)+1; + rcvdSize1= strlen((const char*)protocolRequest.reqJSONPayload)+1; protocolRequest.reqTotalSize = rcvdSize1; } diff --git a/resource/csdk/stack/include/internal/ocserverrequest.h b/resource/csdk/stack/include/internal/ocserverrequest.h index e0d37f3..3b121ca 100644 --- a/resource/csdk/stack/include/internal/ocserverrequest.h +++ b/resource/csdk/stack/include/internal/ocserverrequest.h @@ -100,7 +100,7 @@ OCStackResult AddServerRequest (OCServerRequest ** request, uint16_t coapID, OCQualityOfService qos, unsigned char * query, OCHeaderOption * rcvdVendorSpecificHeaderOptions, unsigned char * reqJSONPayload, OCCoAPToken * requestToken, - OCDevAddr * requesterAddr, unsigned char * resourceUrl, uint32_t reqTotalSize); + OCDevAddr * requesterAddr, unsigned char * resourceUrl, size_t reqTotalSize); #ifdef CA_INT OCStackResult AddServerCARequest (OCServerRequest ** request, uint16_t coapID, @@ -109,7 +109,7 @@ OCStackResult AddServerCARequest (OCServerRequest ** request, uint16_t coapID, OCQualityOfService qos, unsigned char * query, OCHeaderOption * rcvdVendorSpecificHeaderOptions, unsigned char * reqJSONPayload, OCCoAPToken * requestToken, - OCDevAddr * requesterAddr, unsigned char * resourceUrl, uint32_t reqTotalSize, + OCDevAddr * requesterAddr, unsigned char * resourceUrl, size_t reqTotalSize, CAAddress_t *addressInfo, CAConnectivityType_t connectivityType, char *token); #endif diff --git a/resource/csdk/stack/include/internal/ocstackinternal.h b/resource/csdk/stack/include/internal/ocstackinternal.h index a46294c..0eb46d7 100644 --- a/resource/csdk/stack/include/internal/ocstackinternal.h +++ b/resource/csdk/stack/include/internal/ocstackinternal.h @@ -108,7 +108,7 @@ typedef struct { uint16_t reqPacketSize; uint32_t resPacketNum; uint16_t resPacketSize; - uint32_t reqTotalSize; + size_t reqTotalSize; } OCServerProtocolRequest; typedef struct diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c index 55820a1..c98c7ba 100644 --- a/resource/csdk/stack/src/ocserverrequest.c +++ b/resource/csdk/stack/src/ocserverrequest.c @@ -88,10 +88,13 @@ OCStackResult AddServerRequest (OCServerRequest ** request, uint16_t coapID, OCQualityOfService qos, unsigned char * query, OCHeaderOption * rcvdVendorSpecificHeaderOptions, unsigned char * reqJSONPayload, OCCoAPToken * requestToken, - OCDevAddr * requesterAddr, unsigned char * resourceUrl, uint32_t reqTotalSize) + OCDevAddr * requesterAddr, unsigned char * resourceUrl, size_t reqTotalSize) { OCServerRequest * serverRequest = NULL; + //Note: OCServerRequest includes 1 byte for the JSON Payload. payloadSize is calculated + //as the required length of the string, so this will result in enough room for the + //null terminator as well. serverRequest = (OCServerRequest *) OCCalloc(1, sizeof(OCServerRequest) + reqTotalSize - 1); VERIFY_NON_NULL(serverRequest); @@ -118,8 +121,10 @@ OCStackResult AddServerRequest (OCServerRequest ** request, uint16_t coapID, } if(reqJSONPayload) { - memcpy((void *)serverRequest->reqJSONPayload, (void *)reqJSONPayload, - strlen((const char *)reqJSONPayload) + 1); + // destination is at least 1 greater than the source, so a NULL always exists in the + // last character + strncpy((char*)serverRequest->reqJSONPayload, + (const char*)reqJSONPayload, reqTotalSize - 1); } serverRequest->requestComplete = 0; if(requestToken) @@ -157,11 +162,14 @@ OCStackResult AddServerCARequest (OCServerRequest ** request, uint16_t coapID, OCQualityOfService qos, unsigned char * query, OCHeaderOption * rcvdVendorSpecificHeaderOptions, unsigned char * reqJSONPayload, OCCoAPToken * requestToken, - OCDevAddr * requesterAddr, unsigned char * resourceUrl, uint32_t reqTotalSize, + OCDevAddr * requesterAddr, unsigned char * resourceUrl, size_t reqTotalSize, CAAddress_t *addressInfo, CAConnectivityType_t connectivityType, char *token) { OCServerRequest * serverRequest = NULL; + //Note: OCServerRequest includes 1 byte for the JSON Payload. payloadSize is calculated + //as the required length of the string, so this will result in enough room for the + //null terminator as well. serverRequest = (OCServerRequest *) OCCalloc(1, sizeof(OCServerRequest) + reqTotalSize - 1); VERIFY_NON_NULL(serverRequest); @@ -188,8 +196,10 @@ OCStackResult AddServerCARequest (OCServerRequest ** request, uint16_t coapID, } if(reqJSONPayload) { - memcpy((void *)serverRequest->reqJSONPayload, (void *)reqJSONPayload, - strlen((const char *)reqJSONPayload) + 1); + // destination is at least 1 greater than the source, so a NULL always exists in the + // last character + strncpy((char*)serverRequest->reqJSONPayload, + (const char*)reqJSONPayload, reqTotalSize - 1); } serverRequest->requestComplete = 0; if(requestToken) diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index 2877a6d..3324da5 100644 --- a/resource/csdk/stack/src/ocstack.c +++ b/resource/csdk/stack/src/ocstack.c @@ -296,9 +296,15 @@ void HandleCARequests(const CARemoteEndpoint_t* endPoint, const CARequestInfo_t* //copy request payload if (requestInfo->info.payload) { + serverRequest.reqTotalSize = strlen(requestInfo->info.payload) + 1; memcpy (&(serverRequest.reqJSONPayload), requestInfo->info.payload, strlen(requestInfo->info.payload)); } + else + { + serverRequest.reqTotalSize = 1; + } + switch (requestInfo->method) { case CA_GET: -- 2.7.4