[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