[PATCH] better constrain IP-literal validation in ngx_http_validate_host()

Terence Honles terence at honles.com
Mon Dec 17 03:18:19 UTC 2018


# HG changeset patch
# User Terence Honles <terence at honles.com>
# Date 1542840079 28800
#      Wed Nov 21 14:41:19 2018 -0800
# Node ID 0763519f3dcce2c68ccd8894dcc02a4d6114b4c2
# Parent  be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
better constrain IP-literal validation in ngx_http_validate_host()

The existing validation in ngx_http_validate_host() would allow a IP-literal
such as "[127.0.0.1]" which is invalid according to RFC 3986 (See Appendix A.
for the Collected ABNF). This format is intended for IPv6 and IPv-future not
IPv4.

The following changeset does the following:

- Terminates the validation loop if the state is `sw_rest` (If checking this
  every character outweighs the benefit of early termination it can be
  omitted).
- If a "." is found in an IPv6 literal or immediately after the initial "["
  the hostname is determined to be invalid.
- If a "[" is found at any position other than the first character the
  hostname is determined to be invalid.
- Adds a "sw_literal_start" state which branches to "sw_literal_ip_v6" or
  "sw_literal_ip_future"  depending if the character "v" is found at the start
  of the IP-literal.
- If a non hex character (and not ":" because of the explicit case statement)
  is found in an IPv6 literal the hostname is determined to be invalid.
- If the validation loop has not ended in either the "sw_usual" or "sw_rest"
  state the hostname is determined to be invalid (This will occur in an
  unterminated IP-literal)

This changeset *does not* aim to validate IPv-future as it is currently very
permissive and there is no pressing need for this validation.

diff -r be5cb9c67c05 -r 0763519f3dcc src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c Wed Nov 21 20:23:16 2018 +0300
+++ b/src/http/ngx_http_request.c Wed Nov 21 14:41:19 2018 -0800
@@ -1958,12 +1958,14 @@
 static ngx_int_t
 ngx_http_validate_host(ngx_str_t *host, ngx_pool_t *pool, ngx_uint_t alloc)
 {
-    u_char  *h, ch;
+    u_char  *h, c, ch;
     size_t   i, dot_pos, host_len;

     enum {
         sw_usual = 0,
-        sw_literal,
+        sw_literal_start,
+        sw_literal_ip_v6,
+        sw_literal_ip_future,
         sw_rest
     } state;

@@ -1974,13 +1976,16 @@

     state = sw_usual;

-    for (i = 0; i < host->len; i++) {
+    for (i = 0; i < host->len && state != sw_rest; i++) {
         ch = h[i];

         switch (ch) {

         case '.':
-            if (dot_pos == i - 1) {
+            if (state == sw_literal_start
+                || state == sw_literal_ip_v6
+                || dot_pos == i - 1)
+            {
                 return NGX_DECLINED;
             }
             dot_pos = i;
@@ -1995,12 +2000,14 @@

         case '[':
             if (i == 0) {
-                state = sw_literal;
+                state = sw_literal_start;
+            } else {
+                return NGX_DECLINED;
             }
             break;

         case ']':
-            if (state == sw_literal) {
+            if (state == sw_literal_ip_v6 || state == sw_literal_ip_future) {
                 host_len = i + 1;
                 state = sw_rest;
             }
@@ -2015,6 +2022,17 @@
                 return NGX_DECLINED;
             }

+            if (state == sw_literal_start) {
+                state = ch == 'v' ? sw_literal_ip_future : sw_literal_ip_v6;
+            }
+
+            if (state == sw_literal_ip_v6) {
+                c = (u_char) (ch | 0x20);
+                if (!((ch >= '0' && ch <= '9') || (c >= 'a' || c <= 'f'))) {
+                    return NGX_DECLINED;
+                }
+            }
+
             if (ch >= 'A' && ch <= 'Z') {
                 alloc = 1;
             }
@@ -2023,6 +2041,10 @@
         }
     }

+    if (state != sw_usual && state != sw_rest) {
+        return NGX_DECLINED;
+    }
+
     if (dot_pos == host_len - 1) {
         host_len--;
     }


More information about the nginx-devel mailing list