From 8b15d3e1918f8071645e7a59cb277f93dd81cb5d Mon Sep 17 00:00:00 2001 From: George Nash Date: Mon, 5 Dec 2016 15:33:17 -0800 Subject: [PATCH] Prevent out-of-bounds memory access prevent the snprintf function from accessing outside the queryParam char array. If the resulting uri exceeds the max uri length indicate the failure by returning OC_STACK_INVALID_URI from the function. Found using static analysis tool. Change-Id: I81ee4cc932c70942ff65dcf8390529279e36dc4a Signed-off-by: George Nash Reviewed-on: https://gerrit.iotivity.org/gerrit/15173 Reviewed-by: Pawel Winogrodzki Tested-by: jenkins-iotivity Reviewed-by: Dan Mihai --- .../csdk/resource-directory/include/rd_client.h | 6 +++++- resource/csdk/resource-directory/src/rd_client.c | 21 ++++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/resource/csdk/resource-directory/include/rd_client.h b/resource/csdk/resource-directory/include/rd_client.h index b8e90c4..6fafed8 100644 --- a/resource/csdk/resource-directory/include/rd_client.h +++ b/resource/csdk/resource-directory/include/rd_client.h @@ -122,7 +122,11 @@ OCStackResult OCRDDelete(const char *host, OCConnectivityType connectivityType, * received. * @param qos Quality of service. * - * @return ::OC_STACK_OK on success, some other value upon failure. + * @return + * - ::OC_STACK_OK on success + * - ::OC_STACK_INVALID_CALLBACK host, id, or cbData has not been provided + * - ::OC_STACK_INVALID_URI generated URI exceeds MAX_URI_LENGTH try fewer resourceHandles + * - some other value upon failure. */ OCStackResult OCRDDeleteWithDeviceId(const char *host, const unsigned char *id, OCConnectivityType connectivityType, diff --git a/resource/csdk/resource-directory/src/rd_client.c b/resource/csdk/resource-directory/src/rd_client.c index 4710945..be33873 100644 --- a/resource/csdk/resource-directory/src/rd_client.c +++ b/resource/csdk/resource-directory/src/rd_client.c @@ -273,19 +273,34 @@ OCStackResult OCRDDeleteWithDeviceId(const char *host, const unsigned char *id, OIC_LOG_V(DEBUG, TAG, "Delete Resource to RD with device id [%s]", id); char targetUri[MAX_URI_LENGTH] = { 0 }; - snprintf(targetUri, MAX_URI_LENGTH, "%s%s?di=%s", host, OC_RSRVD_RD_URI, id); + int targetUriBufferRequired = snprintf(targetUri, MAX_URI_LENGTH, "%s%s?di=%s", host, OC_RSRVD_RD_URI, id); + if (targetUriBufferRequired >= MAX_URI_LENGTH || targetUriBufferRequired < 0) + { + return OC_STACK_INVALID_URI; + } - uint8_t len = 0; + + int queryLength = 0; char queryParam[MAX_URI_LENGTH] = { 0 }; for (uint8_t j = 0; j < nHandles; j++) { OCResource *handle = (OCResource *) resourceHandles[j]; uint8_t ins = 0; OCGetResourceIns(handle, &ins); - len += snprintf(queryParam + len, MAX_URI_LENGTH, "&ins=%d", ins); + int lenBufferRequired = snprintf(queryParam + queryLength, MAX_URI_LENGTH - queryLength, "&ins=%d", ins); + if (lenBufferRequired >= (MAX_URI_LENGTH - queryLength) || lenBufferRequired < 0) + { + return OC_STACK_INVALID_URI; + } + queryLength += lenBufferRequired; OIC_LOG_V(DEBUG, TAG, "queryParam [%s]", queryParam); } + if (targetUriBufferRequired + queryLength + 1 > MAX_URI_LENGTH) + { + return OC_STACK_INVALID_URI; + } + OICStrcatPartial(targetUri, sizeof(targetUri), queryParam, strlen(queryParam)); OIC_LOG_V(DEBUG, TAG, "Target URI: %s", targetUri); -- 2.7.4