nginx internal DNS cache poisoning

[Igor Sysoev]

On Wed, Sep 16, 2009 at 04:15:14PM -0700, Matthew Dempsky wrote:

> nginx maintains an internal DNS cache for resolved domain names.
> However, when searching the cache, nginx only checks that the crc32 of
> the names match and that the shorter name is a prefix of the longer
> name.  It does not check that the names are equal in length.

Thank you, atached patch fixes the bug.

> One way to exploit this is if nginx is configured as a forward proxy.
> This is an atypical use case, but it has been discussed on the nginx
> mailing list before[1].

> [1] http://marc.info/?l=nginx&m=125257590425747&w=2

Using nginx as forward proxy is not safe thing, at least now, 
because one may return X-Accel-Redirect, etc.


-- 
Igor Sysoev
http://sysoev.ru/en/
-------------- next part --------------
Index: src/core/ngx_resolver.c
===================================================================
--- src/core/ngx_resolver.c	(revision 2462)
+++ src/core/ngx_resolver.c	(working copy)
@@ -1599,7 +1599,6 @@
 ngx_resolver_lookup_name(ngx_resolver_t *r, ngx_str_t *name, uint32_t hash)
 {
     ngx_int_t             rc;
-    size_t                len;
     ngx_rbtree_node_t    *node, *sentinel;
     ngx_resolver_node_t  *rn;
 
@@ -1623,10 +1622,8 @@
         do {
             rn = (ngx_resolver_node_t *) node;
 
-            len = (name->len > (size_t) rn->nlen) ? rn->nlen : name->len;
+            rc = ngx_memn2cmp(name->data, rn->name, name->len, rn->nlen);
 
-            rc = ngx_strncmp(name->data, rn->name, len);
-
             if (rc == 0) {
                 return rn;
             }
@@ -1679,7 +1676,6 @@
 ngx_resolver_rbtree_insert_value(ngx_rbtree_node_t *temp,
     ngx_rbtree_node_t *node, ngx_rbtree_node_t *sentinel)
 {
-    size_t                 len;
     ngx_rbtree_node_t    **p;
     ngx_resolver_node_t   *rn, *rn_temp;
 
@@ -1698,10 +1694,8 @@
             rn = (ngx_resolver_node_t *) node;
             rn_temp = (ngx_resolver_node_t *) temp;
 
-            len = (rn->nlen > rn_temp->nlen) ? rn_temp->nlen : rn->nlen;
-
-            p = (ngx_strncmp(rn->name, rn_temp->name, len) < 0)
-                    ? &temp->left : &temp->right;
+            p = (ngx_memn2cmp(rn->name, rn_temp->name, rn->nlen, rn_temp->nlen)
+                 < 0) ? &temp->left : &temp->right;
         }
 
         if (*p == sentinel) {