commit 5327605caa5863ec9593fd0899425cd971a9d525 Author: Kevin Butler <haqkrs@gmail.com> Date: Tue Sep 3 01:14:43 2013 +0100 Tougher validation for parsing urls from HTTP headers. Fixes #2767. --- changes/bug2767 | 2 ++ src/or/directory.c | 9 ++++++++- src/or/directory.h | 5 +++++ src/test/test_dir.c | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/changes/bug2767 b/changes/bug2767 new file mode 100644 index 0000000..974ce63 --- /dev/null +++ b/changes/bug2767 @@ -0,0 +1,2 @@ + o Minor bugfixes: + - No longer accepting malformed http headers when parsing urls from headers, replies with Bad Request(400). Fixes #2767. \ No newline at end of file diff --git a/src/or/directory.c b/src/or/directory.c index 97305ae..58ce0cf 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1387,7 +1387,7 @@ directory_send_command(dir_connection_t *conn, * so it does. Return 0. * Otherwise, return -1. */ -static int +STATIC int parse_http_url(const char *headers, char **url) { char *s, *start, *tmp; @@ -1416,6 +1416,13 @@ parse_http_url(const char *headers, char **url) } } + /* Check if the header is well formed (next sequence + * should be HTTP/1.X\r\n). Assumes we're supporting 1.0? */ + char *e = (char *)eat_whitespace_no_nl(s); + if (strcmpstart(e, "HTTP/1.") || !(*(e+8) == '\r')) { + return -1; + } + if (s-start < 5 || strcmpstart(start,"/tor/")) { /* need to rewrite it */ *url = tor_malloc(s - start + 5); strlcpy(*url,"/tor", s-start+5); diff --git a/src/or/directory.h b/src/or/directory.h index 41f18a1..0453160 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -118,5 +118,10 @@ download_status_mark_impossible(download_status_t *dl) int download_status_get_n_failures(const download_status_t *dls); +#ifdef TOR_UNIT_TESTS +/* Used only by directory.c and test_dir.c */ +STATIC int parse_http_url(const char *headers, char **url); +#endif + #endif diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 6c2915d..05e13b5 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2364,6 +2364,45 @@ test_dir_fmt_control_ns(void *arg) tor_free(s); } +static void +test_dir_http_handling(void *args) +{ + (void)args; + char *url = NULL; + + /* Parse http url tests: */ + /* Good headers */ + test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.1\r\n" + "Host: example.com\r\n" + "User-Agent: Mozilla/5.0 (Windows;" + " U; Windows NT 6.1; en-US; rv:1.9.1.5)\r\n", + &url), 0); + test_streq(url, "/tor/a/b/c.txt"); + tor_free(url); + + /* Should prepends '/tor/' to url if required */ + test_eq(parse_http_url("GET /a/b/c.txt HTTP/1.1\r\n" + "Host: example.com\r\n" + "User-Agent: Mozilla/5.0 (Windows;" + " U; Windows NT 6.1; en-US; rv:1.9.1.5)\r\n", + &url), 0); + test_streq(url, "/tor/a/b/c.txt"); + tor_free(url); + + /* Bad headers */ + test_eq(parse_http_url("GET /a/b/c.txt\r\n" + "Host: example.com\r\n" + "User-Agent: Mozilla/5.0 (Windows;" + " U; Windows NT 6.1; en-US; rv:1.9.1.5)\r\n", + &url), -1); + tt_assert(!url); + + /* TODO: more http handling tests */ + + done: + ; +} + #define DIR_LEGACY(name) \ { #name, legacy_test_helper, TT_FORK, &legacy_setup, test_dir_ ## name } @@ -2386,6 +2425,7 @@ struct testcase_t dir_tests[] = { DIR_LEGACY(clip_unmeasured_bw_kb_alt), DIR(v2_dir, TT_FORK), DIR(fmt_control_ns, 0), + DIR(http_handling, 0), END_OF_TESTCASES };