diff --git a/Makefile b/Makefile index 780d699..4e84770 100644 --- a/Makefile +++ b/Makefile @@ -126,6 +126,15 @@ clean: .depend: version.h $(CPP) $(CFLAGS) -M $(ALLSRC) > .depend +ALL_TESTS := test_parse_content_length + +test: $(ALL_TESTS) + @set -e; \ + for tst in $(ALL_TESTS); do echo "$$tst"; ./$$tst; done + +test_parse_content_length: test_parse_content_length.c url.c + $(CC) $(CFLAGS) -flto $(INIT_DEFS) -o $@ $< + ifeq (.depend,$(wildcard .depend)) include .depend endif diff --git a/network.c b/network.c index f871a70..1e7755a 100644 --- a/network.c +++ b/network.c @@ -996,7 +996,8 @@ enum return_type http_prepare(void) do { char location_full[500]; char *tmp; - int fd, size; + int fd; + unsigned long size; snprintf(location_full, sizeof(location_full), "Please enter the name or IP address of the HTTP server, " @@ -1032,7 +1033,7 @@ enum return_type http_prepare(void) return RETURN_ERROR; } - log_message("HTTP: size of download %d bytes", size); + log_message("HTTP: size of download %lu bytes", size); results = load_ramdisk_fd(fd, size); close(fd); diff --git a/test_parse_content_length.c b/test_parse_content_length.c new file mode 100644 index 0000000..456bbb5 --- /dev/null +++ b/test_parse_content_length.c @@ -0,0 +1,70 @@ +#include +#include + +void log_message(const char *msg, ...) { + va_list args; + va_start(args, msg); + vfprintf(stderr, msg, args); + fprintf(stderr, "\n"); + fflush(stderr); +} + +#include "url.c" /* yes, include the source file */ + +static int test_parse_content_length(const char *headers, unsigned long expected, int shouldfail) { + int err; + unsigned long val; + err = parse_content_length(headers, &val); + if (err < 0 && !shouldfail) { + log_message("%s: unexpectedly failed on input '%s'", __func__, headers); + return 2; + } + if (err == 0 && shouldfail) { + log_message("%s: unexpectedly passed on invalid input '%s'", __func__, headers); + return 3; + } + if (!shouldfail && val != expected) { + log_message("%s: expected %lu, got %lu on input '%s'", + __func__, expected, val, headers); + return 5; + } + if (!shouldfail && val == expected) { + log_message("%s: OK: '%s'", __func__, headers); + } + return 0; +} + +#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0])) + +int main(int argc, char **argv) { + int i, err = 0; + const char *invalid_inputs[] = { + "", /* empty */ + "Abyrvalg: foobar", /* no Content-Length: */ + "Content-Length: xyz", /* no digits */ + "Content-Length: 12xyz", /* invalid characters */ + "Content-Length: 12 34", /* stray symbols */ + "Content-Length: 123456789012345678901234567890", /* too big */ + }; + const char *valid_inputs[] = { + "Content-Length: 1234", + "Content-Length: 1234", + "Content-Length: 1234 ", + "Content-Length: 1234 ", + "Content-Length: 1234 \r\n\r\n", + }; + + for (i = 0; i < ARRAY_SIZE(invalid_inputs); i++) { + err += test_parse_content_length(invalid_inputs[i], 0, + /* shouldfail = */ 1); + } + for (i = 0; i < ARRAY_SIZE(valid_inputs); i++) { + err += test_parse_content_length(valid_inputs[i], 1234, + /* shouldfail = */ 0); + } + return err; +} + +/* compile command: + gcc -O2 -g -flto -o test_parse_content_length.c test_parse_content_length.c + */ diff --git a/url.c b/url.c index 3b95106..f541ccf 100644 --- a/url.c +++ b/url.c @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -394,11 +395,70 @@ int ftp_end_data_command(int sock) return 0; } +static int parse_content_length(const char *headers, unsigned long *size) { + const char *header_content_length = "Content-Length: "; + const char *hdr = NULL, *ptr = NULL, *start = NULL, *end = NULL; -int http_download_file(char * hostname, char * remotename, int * size) + hdr = strstr(headers, header_content_length); + if (!hdr) { + log_message("%s: error: Content-Length header not found", __func__); + return -1; + } + + start = hdr + strlen(header_content_length); + + errno = 0; + *size = strtoul(start, (char **)&end, 10); + + /* HTTP headers are pretty standard, hence reject a bogus header + * instead of trying to download file of unknown length. + * Also bail out if the file is too long (i.e. size can't be + * represented by unsigned long). + * As a side effect this prevents 32-bit platforms from + * loading large (>= 4GB) files into RAM + */ + if (errno != 0 && errno != ERANGE) { + log_message("%s: error: invalid Content-Length header '%s', parse error: %s", __func__, hdr, strerror(errno)); + return -1; + } + if (ERANGE == errno || *size == ULONG_MAX) { + log_message("%s: error: length '%s' is too big (>= %lu)", __func__, start, ULONG_MAX); + return -1; + } + + /* XXX: on non-numerical inputs strtoul returns 0 *without* + * setting errno. Therefore validate the header manually: only + * a sequence of decimal digits surrounded by whitspace is OK. + * Note: endptr points first non-digit/space character or + * end of the string + */ + for (ptr = end; *ptr; ptr++) { + if (!isspace(*ptr) && !isdigit(*ptr)) { + log_message("%s: error: invalid character %c in Content-Length header '%s'", __func__, *ptr, hdr); + return -1; + } + if (isdigit(*ptr)) { + /* Content-Length: 123 456 + * *end = '4' + */ + log_message("%s: error: stray data '%s' in Content-Length header: '%s'", __func__, ptr, hdr); + return -1; + } + } + + if (*size == 0) { + log_message("%s: length is zero", __func__); + return -1; + } + + return 0; +} + +int http_download_file(char * hostname, char * remotename, unsigned long * size) { char * buf; char headers[4096]; + char *headers_end = NULL; /* points to 1st \r in \r\n\r\n */ char * nextChar = headers; int checkedCode; struct in_addr serverAddress; @@ -406,7 +466,6 @@ int http_download_file(char * hostname, char * remotename, int * size) int sock; int rc; struct sockaddr_in destPort; - char * header_content_length = "Content-Length: "; if ((rc = get_host_address(hostname, &serverAddress))) return rc; @@ -435,7 +494,7 @@ int http_download_file(char * hostname, char * remotename, int * size) *nextChar = '\0'; checkedCode = 0; - while (!strstr(headers, "\r\n\r\n")) { + while (!(headers_end = strstr(headers, "\r\n\r\n"))) { polls.fd = sock; polls.events = POLLIN; rc = poll(&polls, 1, TIMEOUT_SECS*1000); @@ -493,10 +552,14 @@ int http_download_file(char * hostname, char * remotename, int * size) } } - if ((buf = strstr(headers, header_content_length))) - *size = charstar_to_int(buf + strlen(header_content_length)); - else - *size = 0; - + *headers_end = '\0'; /* skip \r\n\r\n */ + if (parse_content_length(headers, size) < 0) { + goto out_err; + } return sock; + +out_err: + *size = 0; /* unknown/error */ + close(sock); + return FTPERR_SERVER_IO_ERROR; } diff --git a/url.h b/url.h index 5a59bd8..8df7238 100644 --- a/url.h +++ b/url.h @@ -26,7 +26,7 @@ int ftp_open_connection(char * host, char * name, char * password, char * proxy) int ftp_start_download(int sock, char * remotename, int * size); int ftp_end_data_command(int sock); -int http_download_file(char * hostname, char * remotename, int * size); +int http_download_file(char * hostname, char * remotename, unsigned long * size); #define FTPERR_BAD_SERVER_RESPONSE -1