Browse Source

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

Kevin Butler 10 years ago
parent
commit
5327605caa
4 changed files with 55 additions and 1 deletions
  1. 2 0
      changes/bug2767
  2. 8 1
      src/or/directory.c
  3. 5 0
      src/or/directory.h
  4. 40 0
      src/test/test_dir.c

+ 2 - 0
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.

+ 8 - 1
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);

+ 5 - 0
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
 

+ 40 - 0
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
 };