Merge remote-tracking branch 'public/bug2767'

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.

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,19 @@ 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);
+  {
+    unsigned minor_ver;
+    char ch;
+    if (2 != tor_sscanf(e, "HTTP/1.%u%c", &minor_ver, &ch)) {
+      return -1;
+    }
+    if (ch != '\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);

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
 

src/test/test_dir.c

@@ -2364,6 +2364,74 @@ 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);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.0\r\n", &url), 0);
+  test_streq(url, "/tor/a/b/c.txt");
+  tor_free(url);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.600\r\n", &url), 0);
+  test_streq(url, "/tor/a/b/c.txt");
+  tor_free(url);
+
+  /* Should prepend '/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 -- no HTTP/1.x*/
+  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);
+
+  /* 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);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt", &url), -1);
+  tt_assert(!url);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.1", &url), -1);
+  tt_assert(!url);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.1x\r\n", &url), -1);
+  tt_assert(!url);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.", &url), -1);
+  tt_assert(!url);
+
+  test_eq(parse_http_url("GET /tor/a/b/c.txt HTTP/1.\r", &url), -1);
+  tt_assert(!url);
+
+ done:
+  tor_free(url);
+}
+
 #define DIR_LEGACY(name)                                                   \
   { #name, legacy_test_helper, TT_FORK, &legacy_setup, test_dir_ ## name }
 
@@ -2386,6 +2454,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
 };