[PATCH] Re: Possible Vulnerability in httpd.c

Jody Bruchon jody at jodybruchon.com
Mon Nov 21 15:50:42 UTC 2016


On 2016-11-21 09:53, Raphael de Carvalho Muniz wrote:
> We understand that the resulting program may have vulnerabilities when 
> the macro "#if ENABLE_FEATURE_HTTPD_RANGES" is enabled, by the fact of 
> utilization that sprintf() function. Second the CWE Project, is the 
> classified by CWE-134, where the use this function that accepts a 
> format string as an argument, but the format string can originate from 
> an external source.
>
> Still second the CWE Project, this vulnerability can cause 
> consequences related a with confidentiality, integrity and 
> availability, like allow for information disclosure which can severely 
> simplify exploitation of the program and the execution of arbitrary code.
>
> We'd very grateful if you could say to us if are you understand this 
> how a vulnerability and if you have a motivation to correct.
>
I'm offering up this patch to fix the problem you've reported. I haven't 
tested it but it should be functionally identical and close the most 
obvious sprintf security holes I found on a cursory examination. Hope 
this helps.

-Jody Bruchon
-------------- next part --------------
>From a6d75d30f172e50e9c46efc9d1d6aba62dd4908b Mon Sep 17 00:00:00 2001
From: Jody Bruchon <jody at jodybruchon.com>
Date: Mon, 21 Nov 2016 10:44:09 -0500
Subject: [PATCH] httpd: change sprintf() to snprintf() to avoid buffer
 overflows

httpd used sprintf() in places where user-provided strings might
be provided. This changes uses of sprintf() to snprintf() with
appropriate size arguments to close the possibility of buffer
overflows.
---
 networking/httpd.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/networking/httpd.c b/networking/httpd.c
index abe83a4..a8cf157 100644
--- a/networking/httpd.c
+++ b/networking/httpd.c
@@ -522,6 +522,7 @@ static void parse_conf(const char *path, int flag)
 	FILE *f;
 	const char *filename;
 	char buf[160];
+	int conf_name_len;
 
 	/* discard old rules */
 	free_Htaccess_IP_list(&ip_a_d);
@@ -539,8 +540,9 @@ static void parse_conf(const char *path, int flag)
 
 	filename = opt_c_configFile;
 	if (flag == SUBDIR_PARSE || filename == NULL) {
-		filename = alloca(strlen(path) + sizeof(HTTPD_CONF) + 2);
-		sprintf((char *)filename, "%s/%s", path, HTTPD_CONF);
+		conf_name_len = strlen(path) + sizeof(HTTPD_CONF) + 2;
+		filename = alloca(conf_name_len);
+		snprintf((char *)filename, conf_name_len, "%s/%s", path, HTTPD_CONF);
 	}
 
 	while ((f = fopen_for_read(filename)) == NULL) {
@@ -957,14 +959,14 @@ static void send_headers(int responseNum)
 
 	/* emit the current date */
 	strftime(tmp_str, sizeof(tmp_str), RFC1123FMT, gmtime(&timer));
-	len = sprintf(iobuf,
+	len = snprintf(iobuf, IOBUF_SIZE,
 			"HTTP/1.0 %d %s\r\nContent-type: %s\r\n"
 			"Date: %s\r\nConnection: close\r\n",
 			responseNum, responseString, mime_type, tmp_str);
 
 #if ENABLE_FEATURE_HTTPD_BASIC_AUTH
 	if (responseNum == HTTP_UNAUTHORIZED) {
-		len += sprintf(iobuf + len,
+		len += snprintf(iobuf + len, IOBUF_SIZE - len,
 				"WWW-Authenticate: Basic realm=\"%s\"\r\n",
 				g_realm);
 	}
@@ -1005,14 +1007,15 @@ static void send_headers(int responseNum)
 		strftime(tmp_str, sizeof(tmp_str), RFC1123FMT, gmtime(&last_mod));
 #if ENABLE_FEATURE_HTTPD_RANGES
 		if (responseNum == HTTP_PARTIAL_CONTENT) {
-			len += sprintf(iobuf + len, "Content-Range: bytes %"OFF_FMT"u-%"OFF_FMT"u/%"OFF_FMT"u\r\n",
+			len += snprintf(iobuf + len, IOBUF_SIZE - len,
+					"Content-Range: bytes %"OFF_FMT"u-%"OFF_FMT"u/%"OFF_FMT"u\r\n",
 					range_start,
 					range_end,
 					file_size);
 			file_size = range_end - range_start + 1;
 		}
 #endif
-		len += sprintf(iobuf + len,
+		len += snprintf(iobuf + len, IOBUF_SIZE - len,
 #if ENABLE_FEATURE_HTTPD_RANGES
 			"Accept-Ranges: bytes\r\n"
 #endif
@@ -1024,12 +1027,12 @@ static void send_headers(int responseNum)
 	}
 
 	if (content_gzip)
-		len += sprintf(iobuf + len, "Content-Encoding: gzip\r\n");
+		len += snprintf(iobuf + len, IOBUF_SIZE - len, "Content-Encoding: gzip\r\n");
 
 	iobuf[len++] = '\r';
 	iobuf[len++] = '\n';
 	if (infoString) {
-		len += sprintf(iobuf + len,
+		len += snprintf(iobuf + len, IOBUF_SIZE - len,
 				"<HTML><HEAD><TITLE>%d %s</TITLE></HEAD>\n"
 				"<BODY><H1>%d %s</H1>\n%s\n</BODY></HTML>\n",
 				responseNum, responseString,
-- 
2.2.1



More information about the busybox mailing list