From 848d66e69daf30d3b64db1450618cd819c370ad4 Mon Sep 17 00:00:00 2001 From: Philipp Matthias Hahn Date: Thu, 1 May 2014 10:41:20 +0200 Subject: [PATCH] Fix broken strncat(..., strlen()) commit 0edaf3361db01425cae0daee7dc3f6039f381a17 replaced several malloc()+strcat() sequences with strncat() using strlen() on the *source* string. This is still vulnerable to overwrite the *target* buffer. While reviewing this commit change the code to directly use snprintf() for concatenating strings and check the length of the target buffer. Signed-off-by: Marcelo Roberto Jimenez --- upnp/src/genlib/net/http/httpreadwrite.c | 7 +++---- upnp/src/urlconfig/urlconfig.c | 21 +++++++-------------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/upnp/src/genlib/net/http/httpreadwrite.c b/upnp/src/genlib/net/http/httpreadwrite.c index 33cdc98..e90aa48 100644 --- a/upnp/src/genlib/net/http/httpreadwrite.c +++ b/upnp/src/genlib/net/http/httpreadwrite.c @@ -481,13 +481,12 @@ int http_SendMessage(SOCKINFO *info, int *TimeOut, const char *fmt, ...) memset(Chunk_Header, 0, sizeof(Chunk_Header)); rc = snprintf(Chunk_Header, - sizeof(Chunk_Header) - strlen ("\r\n"), - "%" PRIzx, num_read); - if (rc < 0 || (unsigned int) rc >= sizeof(Chunk_Header) - strlen ("\r\n")) { + sizeof(Chunk_Header), + "%" PRIzx "\r\n", num_read); + if (rc < 0 || (unsigned int) rc >= sizeof(Chunk_Header)) { RetVal = UPNP_E_INTERNAL_ERROR; goto Cleanup_File; } - strncat(Chunk_Header, "\r\n", strlen ("\r\n")); /* Copy the chunk size header */ memcpy(file_buf - strlen(Chunk_Header), Chunk_Header, diff --git a/upnp/src/urlconfig/urlconfig.c b/upnp/src/urlconfig/urlconfig.c index f3cd7cd..80e6871 100644 --- a/upnp/src/urlconfig/urlconfig.c +++ b/upnp/src/urlconfig/urlconfig.c @@ -143,15 +143,12 @@ static UPNP_INLINE int calc_alias( aliasPtr = alias + 1; else aliasPtr = alias; - new_alias_len = root_len + strlen(temp_str) + strlen(aliasPtr); - alias_temp = malloc(new_alias_len + (size_t)1); + new_alias_len = root_len + strlen(temp_str) + strlen(aliasPtr) + (size_t)1; + alias_temp = malloc(new_alias_len); if (alias_temp == NULL) return UPNP_E_OUTOF_MEMORY; - memset(alias_temp, 0, new_alias_len + (size_t)1); - strncpy(alias_temp, rootPath, root_len); - alias_temp[root_len] = '\0'; - strncat(alias_temp, temp_str, strlen(temp_str)); - strncat(alias_temp, aliasPtr, strlen(aliasPtr)); + memset(alias_temp, 0, new_alias_len); + snprintf(alias_temp, new_alias_len, "%s%s%s", rootPath, temp_str, aliasPtr); *newAlias = alias_temp; return UPNP_E_SUCCESS; @@ -186,14 +183,10 @@ static UPNP_INLINE int calc_descURL( assert(ipPortStr != NULL && strlen(ipPortStr) > 0); assert(alias != NULL && strlen(alias) > 0); - len = strlen(http_scheme) + strlen(ipPortStr) + strlen(alias); - if (len > ((size_t)LINE_SIZE - (size_t)1)) + len = strlen(http_scheme) + strlen(ipPortStr) + strlen(alias) + (size_t)1; + if (len > (size_t)LINE_SIZE) return UPNP_E_URL_TOO_BIG; - strncpy(descURL, http_scheme, strlen(http_scheme)); - descURL[strlen(http_scheme)] = '\0'; - strncat(descURL, ipPortStr, strlen(ipPortStr)); - strncat(descURL, alias, strlen(alias)); - descURL[len] = '\0'; + snprintf(descURL, len, "%s%s%s", http_scheme, ipPortStr, alias); UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__, "desc url: %s\n", descURL);