From e2fa4be291647069758684c6c676aa2d95a5a971 Mon Sep 17 00:00:00 2001 From: Hitarth Thummar <47787284+gtlsgamr@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:47:29 +0530 Subject: [PATCH] Fix memory safety issues Related to #1 Address memory safety issues in `addcomment.c`. * **Memory Allocation Check**: Add a check for `malloc` returning `NULL` in the `udcd` function to prevent dereferencing a `NULL` pointer. * **Memory Deallocation**: Free the allocated memory after use in the `udcd` function to prevent memory leaks. * **Buffer Overflow Prevention**: Add a check to ensure the input length does not exceed the buffer size before using `fread`. * **Input Length Check**: Modify the `sscanf` function to check the length of input strings to prevent buffer overflows. * **Error Handling**: Add error handling for memory allocation failures and free allocated memory in case of errors. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/gtlsgamr/addcomment.c/issues/1?shareId=XXXX-XXXX-XXXX-XXXX). --- addcomment.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/addcomment.c b/addcomment.c index 8f486d8..021d947 100644 --- a/addcomment.c +++ b/addcomment.c @@ -13,6 +13,9 @@ char from_hex(char ch) { /* IMPORTANT: be sure to free() the returned string after use */ char *udcd(char *str) { char *pstr = str, *buf = malloc(strlen(str) + 1), *pbuf = buf; + if (buf == NULL) { + return NULL; + } while (*pstr) { if (*pstr == '%') { if (pstr[1] && pstr[2]) { @@ -37,11 +40,14 @@ int main(void) int InputLength = atoi(getenv("CONTENT_LENGTH") ); int r; - /*InputLength = min( InputLength, sizeof(Buffer)-1 );*/ + if (InputLength >= sizeof(Buffer)) { + printf("ERROR: Input length exceeds buffer size"); + return -1; + } fread( Buffer, InputLength, 1, stdin ); - r = sscanf(Buffer, "alias=%[^&]&url=%[^&]&time=%[^&]&body=%[^&]", alias, url, time, body); + r = sscanf(Buffer, "alias=%24[^&]&url=%499[^&]&time=%99[^&]&body=%4999[^&]", alias, url, time, body); if(r!=4){ printf("ERROR"); @@ -61,7 +67,27 @@ int main(void) return -1; } - fprintf(fp,"\n{\"alias\": \"%s\", \"url\": \"%s\", \"time\": \"%s\", \"body\": \"%s\"},\n",udcd(alias),udcd(url),udcd(time),udcd(body)); + char *decoded_alias = udcd(alias); + char *decoded_url = udcd(url); + char *decoded_time = udcd(time); + char *decoded_body = udcd(body); + + if (decoded_alias == NULL || decoded_url == NULL || decoded_time == NULL || decoded_body == NULL) { + printf("ERROR: Memory allocation failed"); + free(decoded_alias); + free(decoded_url); + free(decoded_time); + free(decoded_body); + fclose(fp); + return -1; + } + + fprintf(fp,"\n{\"alias\": \"%s\", \"url\": \"%s\", \"time\": \"%s\", \"body\": \"%s\"},\n", decoded_alias, decoded_url, decoded_time, decoded_body); + + free(decoded_alias); + free(decoded_url); + free(decoded_time); + free(decoded_body); fclose(fp);