Tougher validation for parsing urls from HTTP headers. Fixes #2767.

4 files changed, 55 insertions, 1 deletions

diff --git a/changes/bug2767 b/changes/bug2767
new file mode 100644
index 000000000..974ce6326
--- /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 97305ae2a..58ce0cf83 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 41f18a172..0453160f7 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 6c2915d09..05e13b574 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
 };