Fix for issue#209: support spaces, commas, quotes and other weirdness in Authorization header.

mongoose.c

@@ -639,24 +639,56 @@ static int mg_snprintf(struct mg_connection *conn, char *buf, size_t buflen,
 }
 
 // Skip the characters until one of the delimiters characters found.
-// 0-terminate resulting word. Skip the rest of the delimiters if any.
+// 0-terminate resulting word. Skip the delimiter and following whitespaces if any.
 // Advance pointer to buffer to the next word. Return found 0-terminated word.
-static char *skip(char **buf, const char *delimiters) {
-  char *p, *begin_word, *end_word, *end_delimiters;
+// Delimiters can be quoted with quotechar.
+static char *skip_quoted(char **buf, const char *delimiters, const char *whitespace, char quotechar) {
+  char *p, *begin_word, *end_word, *end_whitespace;
 
   begin_word = *buf;
   end_word = begin_word + strcspn(begin_word, delimiters);
-  end_delimiters = end_word + strspn(end_word, delimiters);
 
-  for (p = end_word; p < end_delimiters; p++) {
+  /* Check for quotechar */
+  if (end_word > begin_word) {
+    p = end_word - 1;
+    while (*p == quotechar) {
+      /* If there is anything beyond end_word, copy it */
+      if (*end_word == '\0') {
+        *p = '\0';
+        break;
+      } else {
+        size_t end_off = strcspn(end_word + 1, delimiters);
+        memmove (p, end_word, end_off + 1);
+        p += end_off; /* p must correspond to end_word - 1 */
+        end_word += end_off + 1;
+      }
+    }
+    for (p++; p < end_word; p++) {
+      *p = '\0';
+    }
+  }
+
+  if (*end_word == '\0') {
+    *buf = end_word;
+  } else {
+    end_whitespace = end_word + 1 + strspn(end_word + 1, whitespace);
+
+    for (p = end_word; p < end_whitespace; p++) {
       *p = '\0';
     }
 
-  *buf = end_delimiters;
+    *buf = end_whitespace;
+  }
 
   return begin_word;
 }
 
+// Simplified version of skip_quoted without quote char and whitespace == delimiters
+static char *skip(char **buf, const char *delimiters) {
+  return skip_quoted(buf, delimiters, delimiters, 0);
+}
+
+
 // Return HTTP header value, or NULL if not found.
 static const char *get_header(const struct mg_request_info *ri,
                               const char *name) {
@@ -2058,26 +2090,26 @@ static int parse_auth_header(struct mg_connection *conn, char *buf,
   s = buf;
   (void) memset(ah, 0, sizeof(*ah));
 
+  // Parse authorization header
+  for (;;) {
     // Gobble initial spaces
     while (isspace(* (unsigned char *) s)) {
       s++;
     }
-
-  // Parse authorization header
-  for (;;) {
-    name = skip(&s, "=");
-    value = skip(&s, ", ");  // IE uses commas, FF uses spaces
-
-    // Handle commas: Digest username="a", realm="b", ...
-    if (value[strlen(value) - 1] == ',') {
-      value[strlen(value) - 1] = '\0';
+    name = skip_quoted(&s, "=", " ", 0);
+    /* Value is either quote-delimited, or ends at first comma or space. */
+    if (s[0] == '\"') {
+      s++;
+      value = skip_quoted(&s, "\"", " ", '\\');
+      if (s[0] == ',') {
+        s++;
       }
-
-    // Trim double quotes around values
-    if (*value == '"') {
-      value++;
-      value[strlen(value) - 1] = '\0';
-    } else if (*value == '\0') {
+    }
+    else
+    {
+      value = skip_quoted(&s, ", ", " ", 0);  // IE uses commas, FF uses spaces
+    }
+    if (*name == '\0') {
       break;
     }
 
@@ -2532,7 +2564,7 @@ static void parse_http_headers(char **buf, struct mg_request_info *ri) {
   int i;
 
   for (i = 0; i < (int) ARRAY_SIZE(ri->http_headers); i++) {
-    ri->http_headers[i].name = skip(buf, ": ");
+    ri->http_headers[i].name = skip_quoted(buf, ":", " ", 0);
     ri->http_headers[i].value = skip(buf, "\r\n");
     if (ri->http_headers[i].name[0] == '\0')
       break;

test/test.pl

@@ -303,11 +303,19 @@ o("GET /$test_dir_uri/sort/?dd HTTP/1.0\n\n",
 
 unless (scalar(@ARGV) > 0 and $ARGV[0] eq "basic_tests") {
   # Check that .htpasswd file existence trigger authorization
-  write_file("$root/.htpasswd", '');
+  write_file("$root/.htpasswd", 'user with space, " and comma:mydomain.com:5deda12442309cbdcdffc6b2737a894f');
   o("GET /hello.txt HTTP/1.1\n\n", '401 Unauthorized',
     '.htpasswd - triggering auth on file request');
   o("GET / HTTP/1.1\n\n", '401 Unauthorized',
     '.htpasswd - triggering auth on directory request');
+
+  # Test various funky things in an authentication header.
+  o("GET /hello.txt HTTP/1.0\nAuthorization: Digest   eq== empty=\"\", empty2=, quoted=\"blah foo bar, baz\\\"\\\" more\\\"\", unterminatedquoted=\" doesn't stop\n\n",
+    '401 Unauthorized', 'weird auth values should not cause crashes');
+  my $auth_header = "Digest username=\"user with space, \\\" and comma\", ".
+    "realm=\"mydomain.com\", nonce=\"1291376417\", uri=\"/\",".
+    "response=\"e8dec0c2a1a0c8a7e9a97b4b5ea6a6e6\", qop=auth, nc=00000001, cnonce=\"1a49b53a47a66e82\"";
+  o("GET /hello.txt HTTP/1.0\nAuthorization: $auth_header\n\n", 'HTTP/1.1 200 OK', 'GET regular file with auth');
   unlink "$root/.htpasswd";
 
   o("GET /env.cgi HTTP/1.0\n\r\n", 'HTTP/1.1 200 OK', 'GET CGI file');
@@ -378,6 +386,8 @@ unless (scalar(@ARGV) > 0 and $ARGV[0] eq "basic_tests") {
 }
 
 sub do_PUT_test {
+  # This only works because mongoose currently doesn't look at the nonce.
+  # It should really be rejected...
   my $auth_header = "Authorization: Digest  username=guest, ".
   "realm=mydomain.com, nonce=1145872809, uri=/put.txt, ".
   "response=896327350763836180c61d87578037d9, qop=auth, ".