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 <erich.keane@intel.com>

diff --git a/resource/csdk/occoap/src/occoap.c b/resource/csdk/occoap/src/occoap.c
index 38f8c0b..947c8ee 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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: