From 9f8eb1ee7620020e01b3596ac7259d51ebca7a7b Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 22 Nov 2016 02:23:35 +0100 Subject: [PATCH] httpd: explain why we use sprintf and why it should be fine While at it, fix a pathological case where it is not fine: -r REALM with some 8-kbyte long REALM would overflow the buffer. Signed-off-by: Denys Vlasenko --- networking/httpd.c | 56 +++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/networking/httpd.c b/networking/httpd.c index abe83a458..c20642e11 100644 --- a/networking/httpd.c +++ b/networking/httpd.c @@ -926,16 +926,16 @@ static void log_and_exit(void) static void send_headers(int responseNum) { static const char RFC1123FMT[] ALIGN1 = "%a, %d %b %Y %H:%M:%S GMT"; + /* Fixed size 29-byte string. Example: Sun, 06 Nov 1994 08:49:37 GMT */ + char date_str[40]; /* using a bit larger buffer to paranoia reasons */ const char *responseString = ""; const char *infoString = NULL; - const char *mime_type; #if ENABLE_FEATURE_HTTPD_ERROR_PAGES const char *error_page = NULL; #endif unsigned i; time_t timer = time(NULL); - char tmp_str[80]; int len; for (i = 0; i < ARRAY_SIZE(http_response_type); i++) { @@ -948,25 +948,33 @@ static void send_headers(int responseNum) break; } } - /* error message is HTML */ - mime_type = responseNum == HTTP_OK ? - found_mime_type : "text/html"; if (verbose) bb_error_msg("response:%u", responseNum); - /* emit the current date */ - strftime(tmp_str, sizeof(tmp_str), RFC1123FMT, gmtime(&timer)); + /* We use sprintf, not snprintf (it's less code). + * iobuf[] is several kbytes long and all headers we generate + * always fit into those kbytes. + */ + + strftime(date_str, sizeof(date_str), RFC1123FMT, gmtime(&timer)); len = sprintf(iobuf, - "HTTP/1.0 %d %s\r\nContent-type: %s\r\n" - "Date: %s\r\nConnection: close\r\n", - responseNum, responseString, mime_type, tmp_str); + "HTTP/1.0 %d %s\r\n" + "Content-type: %s\r\n" + "Date: %s\r\n" + "Connection: close\r\n", + responseNum, responseString, + /* if it's error message, then it's HTML */ + (responseNum == HTTP_OK ? found_mime_type : "text/html"), + date_str + ); #if ENABLE_FEATURE_HTTPD_BASIC_AUTH if (responseNum == HTTP_UNAUTHORIZED) { len += sprintf(iobuf + len, - "WWW-Authenticate: Basic realm=\"%s\"\r\n", - g_realm); + "WWW-Authenticate: Basic realm=\"%.999s\"\r\n", + g_realm /* %.999s protects from overflowing iobuf[] */ + ); } #endif if (responseNum == HTTP_MOVED_TEMPORARILY) { @@ -981,7 +989,8 @@ static void send_headers(int responseNum) "Location: %s/%s%s\r\n", found_moved_temporarily, (g_query ? "?" : ""), - (g_query ? g_query : "")); + (g_query ? g_query : "") + ); if (len > IOBUF_SIZE-3) len = IOBUF_SIZE-3; } @@ -1002,13 +1011,15 @@ static void send_headers(int responseNum) #endif if (file_size != -1) { /* file */ - strftime(tmp_str, sizeof(tmp_str), RFC1123FMT, gmtime(&last_mod)); + strftime(date_str, sizeof(date_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 += sprintf(iobuf + len, + "Content-Range: bytes %"OFF_FMT"u-%"OFF_FMT"u/%"OFF_FMT"u\r\n", range_start, range_end, - file_size); + file_size + ); file_size = range_end - range_start + 1; } #endif @@ -1016,8 +1027,9 @@ static void send_headers(int responseNum) #if ENABLE_FEATURE_HTTPD_RANGES "Accept-Ranges: bytes\r\n" #endif - "Last-Modified: %s\r\n%s %"OFF_FMT"u\r\n", - tmp_str, + "Last-Modified: %s\r\n" + "%s %"OFF_FMT"u\r\n", + date_str, content_gzip ? "Transfer-length:" : "Content-length:", file_size ); @@ -1031,9 +1043,13 @@ static void send_headers(int responseNum) if (infoString) { len += sprintf(iobuf + len, "%d %s\n" - "

%d %s

\n%s\n\n", + "

%d %s

\n" + "%s\n" + "\n", responseNum, responseString, - responseNum, responseString, infoString); + responseNum, responseString, + infoString + ); } if (DEBUG) { iobuf[len] = '\0'; -- 2.25.1