From 113333afa02e3cf4a417824a1c2d2b763b67cc20 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 6 May 2015 14:43:45 -0700 Subject: [PATCH] Fixes a large number of Klocwork identified issues. A large number of memory/strcpy related issues were fixed, as well as some situations where the code was cleaned up to be a bit easier to read. Change-Id: Ieca48d0bbae02604c2e6e904432178e3ea80a417 Signed-off-by: Erich Keane Reviewed-on: https://gerrit.iotivity.org/gerrit/942 Tested-by: jenkins-iotivity --- .../connectivity/common/src/camutex_pthreads.c | 16 ++++++++++- .../common/src/cathreadpool_pthreads.c | 1 + .../connectivity/src/bt_le_adapter/caleadapter.c | 18 +++++++++++-- resource/csdk/connectivity/src/camessagehandler.c | 9 ------- resource/csdk/connectivity/src/caprotocolmessage.c | 6 ++--- resource/csdk/connectivity/src/caremotehandler.c | 18 ++++++++++++- .../csdk/connectivity/src/ip_adapter/caipserver.c | 18 ++++++++----- .../src/ip_adapter/linux/caipnwmonitor.c | 3 ++- .../csdk/connectivity/test/ca_api_unittest.cpp | 31 ++++++++++++++++++++++ resource/csdk/stack/include/internal/occlientcb.h | 6 +++-- resource/csdk/stack/include/internal/ocresource.h | 3 +++ .../samples/linux/SimpleClientServer/occlient.cpp | 3 ++- .../linux/SimpleClientServer/occlientbasicops.cpp | 4 +++ .../samples/linux/SimpleClientServer/ocserver.cpp | 24 ++++++++++++++--- .../linux/SimpleClientServer/ocserverslow.cpp | 6 +++++ .../csdk/stack/samples/linux/secure/common.cpp | 6 ++--- resource/csdk/stack/samples/linux/secure/common.h | 2 ++ .../samples/linux/secure/ocserverbasicops.cpp | 8 +++++- resource/csdk/stack/src/occollection.c | 8 ++++++ resource/csdk/stack/src/ocobserve.c | 3 ++- resource/csdk/stack/src/ocresource.c | 10 ++++--- resource/csdk/stack/src/ocserverrequest.c | 10 ++++++- resource/csdk/stack/src/ocstack.c | 18 ++++++++++++- resource/csdk/stack/src/oicgroup.c | 14 +++++++--- resource/csdk/stack/test/gtest_helper.h | 2 +- resource/examples/devicediscoveryclient.cpp | 9 +++---- resource/examples/devicediscoveryserver.cpp | 7 ++--- resource/examples/fridgeclient.cpp | 2 +- resource/examples/garageclient.cpp | 4 +-- resource/examples/groupclient.cpp | 4 +-- resource/examples/groupserver.cpp | 4 +-- resource/examples/presenceclient.cpp | 3 ++- resource/examples/roomclient.cpp | 3 ++- resource/examples/simpleclient.cpp | 4 +-- resource/examples/simpleclientHQ.cpp | 4 +-- resource/examples/simpleclientserver.cpp | 2 +- resource/examples/simpleserver.cpp | 3 ++- resource/examples/simpleserverHQ.cpp | 3 ++- resource/examples/threadingsample.cpp | 2 +- resource/include/OCRepresentation.h | 2 +- resource/src/InProcServerWrapper.cpp | 7 +++-- resource/src/OCPlatform_impl.cpp | 2 ++ resource/src/OCRepresentation.cpp | 2 +- resource/unittests/OCResourceTest.cpp | 14 +++++++--- 44 files changed, 249 insertions(+), 79 deletions(-) diff --git a/resource/csdk/connectivity/common/src/camutex_pthreads.c b/resource/csdk/connectivity/common/src/camutex_pthreads.c index c05dc56..fea62f2 100644 --- a/resource/csdk/connectivity/common/src/camutex_pthreads.c +++ b/resource/csdk/connectivity/common/src/camutex_pthreads.c @@ -124,7 +124,11 @@ void ca_mutex_lock(ca_mutex mutex) { int ret = pthread_mutex_lock(&mutexInfo->mutex); assert(0 == ret); - (void)ret; + if(ret != 0) + { + OIC_LOG_V(ERROR, TAG, "Pthread Mutex lock failed: %d", ret); + exit(ret); + } } else { @@ -170,7 +174,13 @@ void ca_mutex_unlock(ca_mutex mutex) if (mutexInfo) { int ret = pthread_mutex_unlock(&mutexInfo->mutex); + assert ( 0 == ret); + if(ret != 0) + { + OIC_LOG_V(ERROR, TAG, "Pthread Mutex unlock failed: %d", ret); + exit(ret); + } (void)ret; } else @@ -191,6 +201,7 @@ ca_cond ca_cond_new(void) { OIC_LOG_V(ERROR, TAG, "%s: Failed to initialize condition variable attribute %d!", __func__, ret); + OICFree(eventInfo); return retVal; } @@ -201,6 +212,8 @@ ca_cond ca_cond_new(void) { OIC_LOG_V(ERROR, TAG, "%s: Failed to set condition variable clock %d!", __func__, ret); + pthread_condattr_destroy(&(eventInfo->condattr)); + OICFree(eventInfo); return retVal; } #endif @@ -212,6 +225,7 @@ ca_cond ca_cond_new(void) else { OIC_LOG_V(ERROR, TAG, "%s: Failed to initialize condition variable %d!", __func__, ret); + pthread_condattr_destroy(&(eventInfo->condattr)); OICFree(eventInfo); } } diff --git a/resource/csdk/connectivity/common/src/cathreadpool_pthreads.c b/resource/csdk/connectivity/common/src/cathreadpool_pthreads.c index d06b0a1..744b1a5 100644 --- a/resource/csdk/connectivity/common/src/cathreadpool_pthreads.c +++ b/resource/csdk/connectivity/common/src/cathreadpool_pthreads.c @@ -185,6 +185,7 @@ void ca_thread_pool_free(ca_thread_pool_t thread_pool) if(!thread_pool) { OIC_LOG(ERROR, TAG, "Invalid parameter thread_pool was NULL"); + return; } ca_mutex_lock(thread_pool->details->list_lock); diff --git a/resource/csdk/connectivity/src/bt_le_adapter/caleadapter.c b/resource/csdk/connectivity/src/bt_le_adapter/caleadapter.c index 2c640ea..282cf88 100644 --- a/resource/csdk/connectivity/src/bt_le_adapter/caleadapter.c +++ b/resource/csdk/connectivity/src/bt_le_adapter/caleadapter.c @@ -553,9 +553,23 @@ CAResult_t CAGetLEInterfaceInformation(CALocalConnectivity_t **info, uint32_t *s return CA_STATUS_FAILED; } - strncpy((*info)->addressInfo.BT.btMacAddress, local_address, strlen(local_address)); + size_t local_address_len = strlen(local_address); + + if(local_address_len >= sizeof(g_localBLEAddress) || + local_address_len >= sizeof((*info)->addressInfo.BT.btMacAddress)) + { + OIC_LOG(ERROR, CALEADAPTER_TAG, "local_address is too long"); + OICFree(*info); + OICFree(local_address); + return CA_STATUS_FAILED; + } + + strncpy((*info)->addressInfo.BT.btMacAddress, local_address, + sizeof((*info)->addressInfo.BT.btMacAddress) - 1); + (*info)->addressInfo.BT.btMacAddress[sizeof((*info)->addressInfo.BT.btMacAddress)-1] = '\0'; ca_mutex_lock(g_bleLocalAddressMutex); - strncpy(g_localBLEAddress, local_address, sizeof(g_localBLEAddress)); + strncpy(g_localBLEAddress, local_address, sizeof(g_localBLEAddress) - 1); + g_localBLEAddress[sizeof(g_localBLEAddress)-1] = '\0'; ca_mutex_unlock(g_bleLocalAddressMutex); (*info)->type = CA_LE; diff --git a/resource/csdk/connectivity/src/camessagehandler.c b/resource/csdk/connectivity/src/camessagehandler.c index 2779c54..66c27ce 100755 --- a/resource/csdk/connectivity/src/camessagehandler.c +++ b/resource/csdk/connectivity/src/camessagehandler.c @@ -399,11 +399,6 @@ static void CAReceivedPacketCallback(CARemoteEndpoint_t *endpoint, void *data, u if (NULL == cadata) { OIC_LOG(ERROR, TAG, "CAReceivedPacketCallback, Memory allocation failed !"); - if (NULL != endpoint && NULL != endpoint->resourceUri) - { - OICFree(endpoint->resourceUri); - } - OICFree(ReqInfo); coap_delete_pdu(pdu); CAAdapterFreeRemoteEndpoint(endpoint); @@ -477,10 +472,6 @@ static void CAReceivedPacketCallback(CARemoteEndpoint_t *endpoint, void *data, u if (NULL == cadata) { OIC_LOG(ERROR, TAG, "CAReceivedPacketCallback, Memory allocation failed !"); - if (NULL != endpoint && NULL != endpoint->resourceUri) - { - OICFree(endpoint->resourceUri); - } OICFree(ResInfo); coap_delete_pdu(pdu); CAAdapterFreeRemoteEndpoint(endpoint); diff --git a/resource/csdk/connectivity/src/caprotocolmessage.c b/resource/csdk/connectivity/src/caprotocolmessage.c index c45dc02..456d423 100644 --- a/resource/csdk/connectivity/src/caprotocolmessage.c +++ b/resource/csdk/connectivity/src/caprotocolmessage.c @@ -59,7 +59,7 @@ #define CA_BUFSIZE (128) #define CA_PDU_MIN_SIZE (4) -#define CA_PORT_BUFFER_SIZE (2) +#define CA_PORT_BUFFER_SIZE (4) static const char COAP_URI_HEADER[] = "coap://[::]/"; @@ -596,7 +596,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, uint32_t *outCode, CAInfo_t * // Make sure there is enough room in the optionResult buffer if ((optionLength + bufLength) < sizeof(optionResult)) { - memcpy(optionResult + optionLength, buf, bufLength); + memcpy(&optionResult[optionLength], buf, bufLength); optionLength += bufLength; } else @@ -652,7 +652,7 @@ CAResult_t CAGetInfoFromPDU(const coap_pdu_t *pdu, uint32_t *outCode, CAInfo_t * // Make sure there is enough room in the optionResult buffer if ((optionLength + bufLength) < sizeof(optionResult)) { - memcpy(optionResult + optionLength, buf, bufLength); + memcpy(&optionResult[optionLength], buf, bufLength); optionLength += bufLength; } else diff --git a/resource/csdk/connectivity/src/caremotehandler.c b/resource/csdk/connectivity/src/caremotehandler.c index 1271fd9..9bf509a 100644 --- a/resource/csdk/connectivity/src/caremotehandler.c +++ b/resource/csdk/connectivity/src/caremotehandler.c @@ -110,7 +110,22 @@ static int32_t getCAAddress(const char *pAddress, CAAddress_t *outAddress) if (isIp) { - strncpy(outAddress->IP.ipAddress, pAddress, ipLen == 0 ? len : ipLen); + if(ipLen && ipLen < sizeof(outAddress->IP.ipAddress)) + { + strncpy(outAddress->IP.ipAddress, pAddress, ipLen); + outAddress->IP.ipAddress[ipLen] = '\0'; + } + else if (!ipLen && len < sizeof(outAddress->IP.ipAddress)) + { + strncpy(outAddress->IP.ipAddress, pAddress, len); + outAddress->IP.ipAddress[len] = '\0'; + } + else + { + OIC_LOG_V(ERROR, TAG, "IP Address too long: %d", ipLen==0 ? len : ipLen); + return -1; + } + if (ipLen > 0) { @@ -176,6 +191,7 @@ CARemoteEndpoint_t *CACreateRemoteEndpointUriInternal(const CAURI_t uri, } memcpy(cloneUri, &uri[startIndex], sizeof(char) * len); + cloneUri[len] = '\0'; // #3. parse address // #4. parse resource uri diff --git a/resource/csdk/connectivity/src/ip_adapter/caipserver.c b/resource/csdk/connectivity/src/ip_adapter/caipserver.c index c2a4ba6..f3ac991 100644 --- a/resource/csdk/connectivity/src/ip_adapter/caipserver.c +++ b/resource/csdk/connectivity/src/ip_adapter/caipserver.c @@ -627,16 +627,19 @@ CAResult_t CAIPStartUnicastServer(const char *localAddress, uint16_t *port, } if (netMask) { - strncpy(info->subNetMask, netMask, strlen(netMask)); + strncpy(info->subNetMask, netMask, sizeof(info->subNetMask) - 1); + info->subNetMask[sizeof(info->subNetMask)-1] = '\0'; OICFree(netMask); } - strncpy(info->ipAddress, localAddress, strlen(localAddress)); + strncpy(info->ipAddress, localAddress, sizeof(info->ipAddress) - 1); + info->ipAddress[sizeof(info->ipAddress) - 1] = '\0'; info->port = *port; info->socketFd = unicastServerFd; info->isSecured = isSecured; info->isServerStarted = true; info->isMulticastServer = false; - strncpy(info->ifAddr, localAddress, strlen(localAddress)); + strncpy(info->ifAddr, localAddress, sizeof(info->ifAddr) - 1); + info->ifAddr[sizeof(info->ifAddr) - 1] = '\0'; CAResult_t res = CAAddServerInfo(g_serverInfoList, info); if (CA_STATUS_OK != res) @@ -726,17 +729,20 @@ CAResult_t CAIPStartMulticastServer(const char *localAddress, const char *multic } if (netMask) { - strncpy(info->subNetMask, netMask, strlen(netMask)); + strncpy(info->subNetMask, netMask, sizeof(info->subNetMask) - 1); + info->subNetMask[sizeof(info->subNetMask) -1] = '\0'; OICFree(netMask); } - strncpy(info->ipAddress, multicastAddress, strlen(multicastAddress)); + strncpy(info->ipAddress, multicastAddress, sizeof(info->ipAddress) - 1); + info->ipAddress[sizeof(info->ipAddress) -1] = '\0'; info->port = multicastPort; info->socketFd = mulicastServerFd; info->isSecured = false; info->isServerStarted = true; info->isMulticastServer = true; - strncpy(info->ifAddr, localAddress, strlen(localAddress)); + strncpy(info->ifAddr, localAddress, sizeof(info->ifAddr)-1); + info->ifAddr[sizeof(info->ifAddr) -1] = '\0'; ret = CAAddServerInfo(g_serverInfoList, info); diff --git a/resource/csdk/connectivity/src/ip_adapter/linux/caipnwmonitor.c b/resource/csdk/connectivity/src/ip_adapter/linux/caipnwmonitor.c index 8c7653e..459b812 100644 --- a/resource/csdk/connectivity/src/ip_adapter/linux/caipnwmonitor.c +++ b/resource/csdk/connectivity/src/ip_adapter/linux/caipnwmonitor.c @@ -136,7 +136,8 @@ static void CAIPGetInterfaceInformation(u_arraylist_t **netInterfaceList) return; } // set interface name - strncpy(netInfo->interfaceName, ifa->ifa_name, strlen(ifa->ifa_name)); + strncpy(netInfo->interfaceName, ifa->ifa_name, sizeof(netInfo->interfaceName) - 1); + netInfo->interfaceName[sizeof(netInfo->interfaceName)-1] = '\0'; // set local ip address strncpy(netInfo->ipAddress, interfaceAddress, strlen(interfaceAddress)); diff --git a/resource/csdk/connectivity/test/ca_api_unittest.cpp b/resource/csdk/connectivity/test/ca_api_unittest.cpp index 55edd55..b3acf37 100755 --- a/resource/csdk/connectivity/test/ca_api_unittest.cpp +++ b/resource/csdk/connectivity/test/ca_api_unittest.cpp @@ -324,6 +324,13 @@ TEST(SendRequestTest, DISABLED_TC_16_Positive_01) int length = strlen(NORMAL_INFO_DATA) + strlen("a/light"); requestData.payload = (CAPayload_t) calloc(length, sizeof(char)); + + if(!requestData.payload) + { + CADestroyToken(tempToken); + FAIL() << "requestData.payload allocation failed"; + } + snprintf(requestData.payload, length, NORMAL_INFO_DATA, "a/light"); requestData.type = CA_MSG_NONCONFIRM; @@ -355,6 +362,13 @@ TEST_F(CATests, SendRequestTestWithNullURI) int length = strlen(NORMAL_INFO_DATA) + strlen("a/light"); requestData.payload = (CAPayload_t) calloc(length, sizeof(char)); + + if(!requestData.payload) + { + CADestroyToken(tempToken); + FAIL() << "requestData.payload allocation failed"; + } + snprintf(requestData.payload, length, NORMAL_INFO_DATA, "a/light"); requestData.type = CA_MSG_NONCONFIRM; @@ -528,6 +542,11 @@ TEST(AdvertiseResourceTest, DISABLED_TC_24_Positive_01) CAHeaderOption_t* headerOpt; headerOpt = (CAHeaderOption_t *) calloc(1, optionNum * sizeof(CAHeaderOption_t)); + if(!headerOpt) + { + FAIL() <<"Allocation for headerOpt failed"; + } + char* tmpOptionData1 = (char *) "Hello"; size_t tmpOptionDataLen = (strlen(tmpOptionData1) < CA_MAX_HEADER_OPTION_DATA_LENGTH) ? strlen(tmpOptionData1) : CA_MAX_HEADER_OPTION_DATA_LENGTH - 1; @@ -561,6 +580,11 @@ TEST_F(CATests, AdvertiseResourceTest) CAHeaderOption_t* headerOpt; headerOpt = (CAHeaderOption_t *) calloc(1, optionNum * sizeof(CAHeaderOption_t)); + if(!headerOpt) + { + FAIL() << "Allocation for headerOpt failed"; + } + char* tmpOptionData1 = (char *) "Hello"; size_t tmpOptionDataLen = (strlen(tmpOptionData1) < CA_MAX_HEADER_OPTION_DATA_LENGTH) ? strlen(tmpOptionData1) : CA_MAX_HEADER_OPTION_DATA_LENGTH - 1; @@ -642,6 +666,11 @@ TEST(SendRequestToAllTest, DISABLED_TC_31_Positive_01) CACreateRemoteEndpoint(uri, CA_IPV4, &tempRep); CAGroupEndpoint_t *group = NULL; group = (CAGroupEndpoint_t *) malloc(sizeof(CAGroupEndpoint_t)); + if(!group) + { + FAIL() << "Allocation for group failed"; + } + group->transportType = tempRep->transportType; group->resourceUri = tempRep->resourceUri; @@ -714,6 +743,8 @@ CAResult_t checkGetNetworkInfo() CAResult_t res = CAGetNetworkInformation(&tempInfo, &tempSize); + free(tempInfo); + if (CA_STATUS_OK == res || CA_ADAPTER_NOT_ENABLED == res || CA_NOT_SUPPORTED == res) { diff --git a/resource/csdk/stack/include/internal/occlientcb.h b/resource/csdk/stack/include/internal/occlientcb.h index be296f3..15a7862 100644 --- a/resource/csdk/stack/include/internal/occlientcb.h +++ b/resource/csdk/stack/include/internal/occlientcb.h @@ -22,9 +22,9 @@ #ifndef OC_CLIENT_CB #define OC_CLIENT_CB -#include +#include "ocstack.h" -#include +#include "ocresource.h" #include "cacommon.h" typedef struct OCPresence @@ -42,6 +42,8 @@ typedef struct OCMulticastNode struct OCMulticastNode * next; } OCMulticastNode; +typedef struct resourcetype_t OCResourceType; + typedef struct ClientCB { // callback method defined in application address space OCClientResponseHandler callBack; diff --git a/resource/csdk/stack/include/internal/ocresource.h b/resource/csdk/stack/include/internal/ocresource.h index 9fc21c8..acd6de4 100755 --- a/resource/csdk/stack/include/internal/ocresource.h +++ b/resource/csdk/stack/include/internal/ocresource.h @@ -21,6 +21,9 @@ #ifndef OCRESOURCE_H_ #define OCRESOURCE_H_ +#include "ocstackconfig.h" +#include "occlientcb.h" + #define OC_OBSERVER_NOT_INTERESTED (0) #define OC_OBSERVER_STILL_INTERESTED (1) #define OC_OBSERVER_FAILED_COMM (2) diff --git a/resource/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp b/resource/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp index 4d36a96..c6f175f 100644 --- a/resource/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp +++ b/resource/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp @@ -667,7 +667,8 @@ int InitDeviceDiscovery(OCQualityOfService qos) else { strncpy(szQueryUri, MULTICAST_DEVICE_DISCOVERY_QUERY, - (strlen(MULTICAST_DEVICE_DISCOVERY_QUERY) + 1)); + sizeof(szQueryUri) -1 ); + szQueryUri[sizeof(szQueryUri) -1] = '\0'; } if(UNICAST_DISCOVERY) diff --git a/resource/csdk/stack/samples/linux/SimpleClientServer/occlientbasicops.cpp b/resource/csdk/stack/samples/linux/SimpleClientServer/occlientbasicops.cpp index 1f6051d..f844666 100644 --- a/resource/csdk/stack/samples/linux/SimpleClientServer/occlientbasicops.cpp +++ b/resource/csdk/stack/samples/linux/SimpleClientServer/occlientbasicops.cpp @@ -556,6 +556,9 @@ void collectUniqueResource(const OCClientResponse * clientResponse) != OC_STACK_OK) { OC_LOG(ERROR, TAG, "Error while parsing JSON payload in OCClientResponse"); + OCFree(sid); + OCFree(uri); + return; } int i; @@ -573,6 +576,7 @@ void collectUniqueResource(const OCClientResponse * clientResponse) } } + OCFree(sid); OCFree(uri); } diff --git a/resource/csdk/stack/samples/linux/SimpleClientServer/ocserver.cpp b/resource/csdk/stack/samples/linux/SimpleClientServer/ocserver.cpp index 078b0d1..f4eb589 100644 --- a/resource/csdk/stack/samples/linux/SimpleClientServer/ocserver.cpp +++ b/resource/csdk/stack/samples/linux/SimpleClientServer/ocserver.cpp @@ -110,7 +110,13 @@ char* constructJsonResponse (OCEntityHandlerRequest *ehRequest) if(OC_REST_PUT == ehRequest->method) { // Get cJSON pointer to query - cJSON *putJson = cJSON_Parse((char *)ehRequest->reqJSONPayload); + cJSON *putJson = cJSON_Parse(ehRequest->reqJSONPayload); + + if(!putJson) + { + OC_LOG_V(ERROR, TAG, "Failed to parse JSON: %s", ehRequest->reqJSONPayload); + return NULL; + } // Get root of JSON payload, then the 1st resource. cJSON* carrier = cJSON_GetObjectItem(putJson, "oc"); @@ -187,17 +193,21 @@ OCEntityHandlerResult ProcessGetRequest (OCEntityHandlerRequest *ehRequest, char *payload, uint16_t maxPayloadSize) { OCEntityHandlerResult ehResult; - bool queryPassed = checkIfQueryForPowerPassed(ehRequest->query); // Empty payload if the query has no match. if (queryPassed) { char *getResp = constructJsonResponse(ehRequest); + if(!getResp) + { + OC_LOG(ERROR, TAG, "constructJsonResponse failed"); + return OC_EH_ERROR; + } - if (maxPayloadSize > strlen ((char *)getResp)) + if (maxPayloadSize > strlen (getResp)) { - strncpy(payload, getResp, strlen((char *)getResp)); + strncpy(payload, getResp, strlen(getResp)); ehResult = OC_EH_OK; } else @@ -223,6 +233,12 @@ OCEntityHandlerResult ProcessPutRequest (OCEntityHandlerRequest *ehRequest, OCEntityHandlerResult ehResult; char *putResp = constructJsonResponse(ehRequest); + if(!putResp) + { + OC_LOG(ERROR, TAG, "Failed to construct Json response"); + return OC_EH_ERROR; + } + if (maxPayloadSize > strlen ((char *)putResp)) { strncpy(payload, putResp, strlen((char *)putResp)); diff --git a/resource/csdk/stack/samples/linux/SimpleClientServer/ocserverslow.cpp b/resource/csdk/stack/samples/linux/SimpleClientServer/ocserverslow.cpp index 533787a..542abdc 100644 --- a/resource/csdk/stack/samples/linux/SimpleClientServer/ocserverslow.cpp +++ b/resource/csdk/stack/samples/linux/SimpleClientServer/ocserverslow.cpp @@ -120,6 +120,12 @@ void ProcessGetRequest (OCEntityHandlerRequest *ehRequest) { OC_LOG(INFO, TAG, "Entering ProcessGetRequest"); char *getResp = constructJsonResponse(ehRequest); + + if(!getResp) + { + OC_LOG(ERROR, TAG, "Failed to constructJsonResponse"); + return; + } OC_LOG(INFO, TAG, "After constructJsonResponse"); OCEntityHandlerResponse response; diff --git a/resource/csdk/stack/samples/linux/secure/common.cpp b/resource/csdk/stack/samples/linux/secure/common.cpp index 5ee088d..7c04494 100644 --- a/resource/csdk/stack/samples/linux/secure/common.cpp +++ b/resource/csdk/stack/samples/linux/secure/common.cpp @@ -17,10 +17,10 @@ // limitations under the License. // //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= +#include "common.h" -#include -#include -#include +#include "ocsecurity.h" +#include "logger.h" #include #include #include diff --git a/resource/csdk/stack/samples/linux/secure/common.h b/resource/csdk/stack/samples/linux/secure/common.h index d9f56e6..ac73c1d 100644 --- a/resource/csdk/stack/samples/linux/secure/common.h +++ b/resource/csdk/stack/samples/linux/secure/common.h @@ -21,6 +21,8 @@ #ifndef OCSAMPLE_COMMON_H_ #define OCSAMPLE_COMMON_H_ +#include "ocstack.h" + /* Get the result in string format. */ const char *getResult(OCStackResult result); diff --git a/resource/csdk/stack/samples/linux/secure/ocserverbasicops.cpp b/resource/csdk/stack/samples/linux/secure/ocserverbasicops.cpp index 582902c..d63207b 100644 --- a/resource/csdk/stack/samples/linux/secure/ocserverbasicops.cpp +++ b/resource/csdk/stack/samples/linux/secure/ocserverbasicops.cpp @@ -68,7 +68,13 @@ char* constructJsonResponse (OCEntityHandlerRequest *ehRequest) if(OC_REST_PUT == ehRequest->method) { - cJSON *putJson = cJSON_Parse((char *)ehRequest->reqJSONPayload); + cJSON *putJson = cJSON_Parse(ehRequest->reqJSONPayload); + if(!putJson) + { + OC_LOG_V(ERROR, TAG, "Failed to parse JSON: %s", ehRequest->reqJSONPayload); + return NULL; + } + currLEDResource->state = ( !strcmp(cJSON_GetObjectItem(putJson,"state")->valuestring , "on") ? true:false); currLEDResource->power = cJSON_GetObjectItem(putJson,"power")->valuedouble; diff --git a/resource/csdk/stack/src/occollection.c b/resource/csdk/stack/src/occollection.c index 3ac8af0..622288e 100644 --- a/resource/csdk/stack/src/occollection.c +++ b/resource/csdk/stack/src/occollection.c @@ -25,6 +25,7 @@ // For POSIX.1-2001 base specification, // Refer http://pubs.opengroup.org/onlinepubs/009695399/ #define _POSIX_C_SOURCE 200112L +#include "occollection.h" #include #include "ocstack.h" #include "ocstackinternal.h" @@ -236,6 +237,13 @@ static OCStackResult BuildRootResourceJSON(OCResource *resource, { cJSON_AddItemToObject (resObj, OC_RSRVD_HREF, cJSON_CreateString(resource->uri)); jsonStr = cJSON_PrintUnformatted (resObj); + + if(!jsonStr) + { + cJSON_Delete(resObj); + return OC_STACK_NO_MEMORY; + } + jsonLen = strlen(jsonStr); if (jsonLen < *remaining) { diff --git a/resource/csdk/stack/src/ocobserve.c b/resource/csdk/stack/src/ocobserve.c index c5648b7..e7214c1 100644 --- a/resource/csdk/stack/src/ocobserve.c +++ b/resource/csdk/stack/src/ocobserve.c @@ -260,13 +260,14 @@ OCStackResult SendListObserverNotification (OCResource * resource, { OCEntityHandlerResponse ehResponse = {}; ehResponse.ehResult = OC_EH_OK; - ehResponse.payload = (char *) OCMalloc(MAX_RESPONSE_LENGTH); + ehResponse.payload = (char *) OCMalloc(MAX_RESPONSE_LENGTH + 1); if(!ehResponse.payload) { FindAndDeleteServerRequest(request); continue; } strncpy(ehResponse.payload, notificationJSONPayload, MAX_RESPONSE_LENGTH-1); + ehResponse.payload[MAX_RESPONSE_LENGTH] = '\0'; ehResponse.payloadSize = strlen(ehResponse.payload) + 1; ehResponse.persistentBufferFlag = 0; ehResponse.requestHandle = (OCRequestHandle) request; diff --git a/resource/csdk/stack/src/ocresource.c b/resource/csdk/stack/src/ocresource.c index 420aa42..190d2fa 100644 --- a/resource/csdk/stack/src/ocresource.c +++ b/resource/csdk/stack/src/ocresource.c @@ -25,10 +25,8 @@ // For POSIX.1-2001 base specification, // Refer http://pubs.opengroup.org/onlinepubs/009695399/ #define _POSIX_C_SOURCE 200112L +#include "ocresource.h" #include -#include "ocstack.h" -#include "ocstackconfig.h" -#include "ocstackinternal.h" #include "ocresourcehandler.h" #include "ocobserve.h" #include "occollection.h" @@ -290,6 +288,12 @@ BuildVirtualResourceResponse(const OCResource *resourcePtr, uint8_t filterOn, } jsonStr = cJSON_PrintUnformatted (resObj); + if(!jsonStr) + { + cJSON_Delete(resObj); + return OC_STACK_NO_MEMORY; + } + jsonLen = strlen(jsonStr); if (jsonLen < *remaining) { diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c index 99611d7..c3dabad 100644 --- a/resource/csdk/stack/src/ocserverrequest.c +++ b/resource/csdk/stack/src/ocserverrequest.c @@ -455,6 +455,13 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) return OC_STACK_ERROR; } + if(ehResponse->payloadSize >= (MAX_RESPONSE_LENGTH))// - OC_JSON_PREFIX_LEN - OC_JSON_SUFFIX_LEN)) + { + OC_LOG_V(ERROR, TAG, "Response payload size was too large. Max is %hu, payload size was %hu", + (MAX_RESPONSE_LENGTH - OC_JSON_PREFIX_LEN - OC_JSON_SUFFIX_LEN), ehResponse->payloadSize); + return OC_STACK_INVALID_PARAM; + } + OCServerRequest *serverRequest = (OCServerRequest *)ehResponse->requestHandle; // Copy the address @@ -517,6 +524,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) if(!responseInfo.info.options) { OC_LOG(FATAL, TAG, PCF("options is NULL")); + OCFree(responseInfo.info.token); return OC_STACK_NO_MEMORY; } @@ -548,7 +556,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) responseInfo.info.options = NULL; } - char payload[MAX_RESPONSE_LENGTH] = {}; + char payload[MAX_RESPONSE_LENGTH + OC_JSON_PREFIX_LEN + OC_JSON_SUFFIX_LEN] = {}; // Put the JSON prefix and suffix around the payload strcpy(payload, (const char *)OC_JSON_PREFIX); diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index fae4aba..44ed5ac 100644 --- a/resource/csdk/stack/src/ocstack.c +++ b/resource/csdk/stack/src/ocstack.c @@ -1505,6 +1505,7 @@ void HandleCARequests(const CARemoteEndpoint_t* endPoint, const CARequestInfo_t* requestInfo->info.type, requestInfo->info.numOptions, requestInfo->info.options, requestInfo->info.token, requestInfo->info.tokenLength); + OCFree(serverRequest.requestToken); return; } serverRequest.numRcvdVendorSpecificHeaderOptions = tempNum; @@ -1897,6 +1898,14 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ if(method == OC_REST_PRESENCE) { result = getQueryFromUri(requiredUri, &query, &newUri); + + if(result != OC_STACK_OK) + { + OC_LOG_V(ERROR, TAG, "Invalid Param from getQueryFromUri: %d, URI is %s", + result, requiredUri); + goto exit; + } + if(query) { result = getResourceType((char *) query, &resourceType); @@ -1997,6 +2006,7 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ numOptions, OC_OBSERVE_REGISTER); if (result != OC_STACK_OK) { + CADestroyToken(token); goto exit; } requestData.numOptions = numOptions + 1; @@ -2009,7 +2019,6 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ requestData.payload = (char *)request; requestInfo.info = requestData; - CATransportType_t caConType; result = OCToCATransportType((OCConnectivityType) conType, &caConType); @@ -2028,6 +2037,7 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ if(!grpEnd.resourceUri) { result = OC_STACK_NO_MEMORY; + CADestroyToken(token); goto exit; } strncpy(grpEnd.resourceUri, requiredUri, (uriLen + 1)); @@ -2086,6 +2096,7 @@ exit: } CADestroyRemoteEndpoint(endpoint); OCFree(grpEnd.resourceUri); + if (requestData.options && requestData.numOptions > 0) { if ((method == OC_REST_OBSERVE) || (method == OC_REST_OBSERVE_ALL)) @@ -3765,6 +3776,11 @@ OCStackResult getQueryFromUri(const char * uri, char** query, char ** uriWithout } strncpy(*uriWithoutQuery, uri, uriWithoutQueryLen); } + else + { + return OC_STACK_INVALID_PARAM; + } + if (queryLen) { *query = (char *) OCCalloc(queryLen + 1, 1); diff --git a/resource/csdk/stack/src/oicgroup.c b/resource/csdk/stack/src/oicgroup.c index e2642e8..66fce6c 100755 --- a/resource/csdk/stack/src/oicgroup.c +++ b/resource/csdk/stack/src/oicgroup.c @@ -1056,8 +1056,16 @@ OCStackResult BuildCollectionGroupActionJSONResponse( char *jsonResponse; - ExtractKeyValueFromRequest((char *) ehRequest->reqJSONPayload, &doWhat, - &details); + stackRet = ExtractKeyValueFromRequest((char *) ehRequest->reqJSONPayload, + &doWhat, &details); + + if(stackRet != OC_STACK_OK) + { + OC_LOG_V(ERROR, TAG, "ExtractKeyValueFromRequest failed: %d", stackRet); + return stackRet; + } + + stackRet = OC_STACK_ERROR; cJSON *json; cJSON *format; @@ -1270,7 +1278,7 @@ OCStackResult BuildCollectionGroupActionJSONResponse( { cJSON_AddStringToObject(format, ACTIONSET, plainText); } - + OCFree(plainText); stackRet = OC_STACK_OK; } } diff --git a/resource/csdk/stack/test/gtest_helper.h b/resource/csdk/stack/test/gtest_helper.h index a9bb2b4..97ce5f9 100644 --- a/resource/csdk/stack/test/gtest_helper.h +++ b/resource/csdk/stack/test/gtest_helper.h @@ -138,7 +138,7 @@ namespace iotivity try { throw std::runtime_error("deadman timer expired"); } - catch (std::exception &e) + catch (std::exception&) { std::terminate(); } diff --git a/resource/examples/devicediscoveryclient.cpp b/resource/examples/devicediscoveryclient.cpp index 20bc0aa..e3dba58 100644 --- a/resource/examples/devicediscoveryclient.cpp +++ b/resource/examples/devicediscoveryclient.cpp @@ -146,7 +146,7 @@ int main(int argc, char* argv[]) { std::cout << "Invalid connectivity type selected. Using default WIFI" << std::endl; } } - catch(std::exception& e) + catch(std::exception&) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; } @@ -183,14 +183,11 @@ int main(int argc, char* argv[]) { std::mutex blocker; std::condition_variable cv; std::unique_lock lock(blocker); - while(true) - { - cv.wait(lock); - } + cv.wait(lock, []{return false;}); }catch(OCException& e) { - //log(e.what()); + std::cerr << "Failure in main thread: "< lock(blocker); - while(true) - { - cv.wait(lock); - } + cv.wait(lock, []{return false;}); // No explicit call to stop the platform. // When OCPlatform::destructor is invoked, internally we do platform cleanup diff --git a/resource/examples/fridgeclient.cpp b/resource/examples/fridgeclient.cpp index db03240..259aec4 100644 --- a/resource/examples/fridgeclient.cpp +++ b/resource/examples/fridgeclient.cpp @@ -296,7 +296,7 @@ int main(int argc, char* argv[]) std::cout << "Invalid connectivity type selected. Using default WIFI" << std::endl; } } - catch(std::exception& e) + catch(std::exception&) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; } diff --git a/resource/examples/garageclient.cpp b/resource/examples/garageclient.cpp index 680f3c4..8dcc6b2 100644 --- a/resource/examples/garageclient.cpp +++ b/resource/examples/garageclient.cpp @@ -280,7 +280,7 @@ void foundResource(std::shared_ptr resource) } catch(std::exception& e) { - //log(e.what()); + std::cerr << "Exception in foundResource: "<< e.what()< resource) } catch (std::exception& e) { - std::cout << "" << std::endl; + std::cerr << "Exception in foundResource: "<< e.what() << std::endl; } } @@ -203,7 +203,7 @@ int main(int argc, char* argv[]) std::cout << "Invalid connectivity type selected. Using default WIFI" << std::endl; } } - catch(exception& e) + catch(exception&) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; } diff --git a/resource/examples/groupserver.cpp b/resource/examples/groupserver.cpp index 4bfe8be..fc4aaef 100644 --- a/resource/examples/groupserver.cpp +++ b/resource/examples/groupserver.cpp @@ -71,7 +71,7 @@ void foundResource(std::shared_ptr< OCResource > resource) } catch (std::exception& e) { - std::cout << "" << std::endl; + std::cout << "Exception in foundResource:"<< e.what() << std::endl; } } @@ -110,7 +110,7 @@ int main(int argc, char* argv[]) std::cout << "Invalid connectivity type selected. Using default WIFI" << std::endl; } } - catch(exception& e) + catch(exception&) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; } diff --git a/resource/examples/presenceclient.cpp b/resource/examples/presenceclient.cpp index eecabc0..5a3c686 100644 --- a/resource/examples/presenceclient.cpp +++ b/resource/examples/presenceclient.cpp @@ -191,6 +191,7 @@ void foundResource(std::shared_ptr resource) } catch(std::exception& e) { + std::cerr << "Exception in foundResource: "<< e.what() << std::endl; //log(e.what()); } } @@ -244,7 +245,7 @@ int main(int argc, char* argv[]) { } } } - catch(std::exception& e) + catch(std::exception& ) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; diff --git a/resource/examples/roomclient.cpp b/resource/examples/roomclient.cpp index eae63e7..6d5c759 100644 --- a/resource/examples/roomclient.cpp +++ b/resource/examples/roomclient.cpp @@ -215,6 +215,7 @@ void foundResource(std::shared_ptr resource) } catch(std::exception& e) { + std::cerr << "Exception in foundResource: "<< e.what() < resource) } catch(std::exception& e) { - //log(e.what()); + std::cerr << "Exception in foundResource: "<< e.what() << std::endl; } } @@ -464,7 +464,7 @@ int main(int argc, char* argv[]) { return -1; } } - catch(std::exception& e) + catch(std::exception& ) { std::cout << "<===Invalid input arguments===>\n\n"; return -1; diff --git a/resource/examples/simpleclientHQ.cpp b/resource/examples/simpleclientHQ.cpp index bf20e1c..98bc93d 100644 --- a/resource/examples/simpleclientHQ.cpp +++ b/resource/examples/simpleclientHQ.cpp @@ -363,7 +363,7 @@ void foundResource(std::shared_ptr resource) } catch(std::exception& e) { - //log(e.what()); + std::cerr << "Exception in foundResource: "<< e.what() < resourceResponse(new OCResourceResponse()); + std::shared_ptr resourceResponse = + {std::make_shared()}; resourceResponse->setErrorCode(200); resourceResponse->setResourceRepresentation(lightPtr->get(), DEFAULT_INTERFACE); diff --git a/resource/examples/simpleserverHQ.cpp b/resource/examples/simpleserverHQ.cpp index d80d0ed..3239291 100644 --- a/resource/examples/simpleserverHQ.cpp +++ b/resource/examples/simpleserverHQ.cpp @@ -380,7 +380,8 @@ void * ChangeLightRepresentation (void *param) if(isListOfObservers) { - std::shared_ptr resourceResponse(new OCResourceResponse()); + std::shared_ptr resourceResponse = + std::make_shared(); resourceResponse->setErrorCode(200); resourceResponse->setResourceRepresentation(lightPtr->get(), DEFAULT_INTERFACE); diff --git a/resource/examples/threadingsample.cpp b/resource/examples/threadingsample.cpp index b7b5d09..95e0489 100644 --- a/resource/examples/threadingsample.cpp +++ b/resource/examples/threadingsample.cpp @@ -364,7 +364,7 @@ int main(int argc, char* argv[]) std::cout << "Invalid connectivity type selected. Using default WIFI" << std::endl; } } - catch(std::exception& e) + catch(std::exception& ) { std::cout << "Invalid input argument. Using WIFI as connectivity type" << std::endl; } diff --git a/resource/include/OCRepresentation.h b/resource/include/OCRepresentation.h index 97d8924..63fbd62 100644 --- a/resource/include/OCRepresentation.h +++ b/resource/include/OCRepresentation.h @@ -21,7 +21,7 @@ /** * @file * - * This file contains the declaration of classes and its members related + * This file contains the declaration of classes and its members related * to OCRepresentation. */ diff --git a/resource/src/InProcServerWrapper.cpp b/resource/src/InProcServerWrapper.cpp index 927eeff..8f872f2 100644 --- a/resource/src/InProcServerWrapper.cpp +++ b/resource/src/InProcServerWrapper.cpp @@ -291,9 +291,11 @@ namespace OC result = OCProcess(); } - // ...the value of variable result is simply ignored for now. if(OC_STACK_ERROR == result) - ; + { + oclog() << "OCProcess failed with result " << result < #include #include diff --git a/resource/src/OCRepresentation.cpp b/resource/src/OCRepresentation.cpp index 5fc23ff..6f73435 100644 --- a/resource/src/OCRepresentation.cpp +++ b/resource/src/OCRepresentation.cpp @@ -312,7 +312,7 @@ namespace OC { ar(v); } - catch(cereal::Exception& e) + catch(cereal::Exception&) { ar.setNextName(nullptr); // Loading a key that doesn't exist results in an exception diff --git a/resource/unittests/OCResourceTest.cpp b/resource/unittests/OCResourceTest.cpp index ae4b534..daf4517 100644 --- a/resource/unittests/OCResourceTest.cpp +++ b/resource/unittests/OCResourceTest.cpp @@ -53,8 +53,16 @@ namespace OCResourceTest std::vector types = {"intel.rpost"}; std::vector ifaces = {DEFAULT_INTERFACE}; - return OCPlatform::constructResourceObject(host, uri, + auto ret = OCPlatform::constructResourceObject(host, uri, connectivityType, false, types, ifaces); + + if(!ret) + { + ADD_FAILURE() << "ConstructResourceObject result was null"; + throw std::runtime_error("ConstructResourceObject result was null"); + } + + return ret; } //Get Test @@ -252,7 +260,7 @@ namespace OCResourceTest OCRepresentation rep; EXPECT_ANY_THROW( resource->put(nullptr, DEFAULT_INTERFACE, rep, QueryParamsMap(), &onGetPut)); - HeaderOptions headerOptions = {}; + HeaderOptions headerOptions; onGetPut(headerOptions, rep, OC_STACK_OK); } @@ -492,7 +500,7 @@ namespace OCResourceTest { OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000", "/resource"); EXPECT_TRUE(resource != NULL); - HeaderOptions headerOptions = {}; + HeaderOptions headerOptions; EXPECT_NO_THROW(resource->setHeaderOptions(headerOptions)); EXPECT_NO_THROW(resource->unsetHeaderOptions()); } -- 2.7.4