bug in autoindex module

[Maxim Dounin]

Hello!

On Thu, Dec 31, 2009 at 01:55:31AM +0300, Maxim Dounin wrote:

> Hello!
> 
> On Thu, Dec 31, 2009 at 12:45:18AM +0700, Edho P Arief wrote:
> 
> > On Tue, Dec 29, 2009 at 5:20 PM, Edho P Arief <edhoprima at gmail.com> wrote:
> > > Don't know if found by someone else, but I find this bug today in
> > > autoindex module.
> > >
> > > Basically, the file/dirname is not escaped properly.
> > >
> > > To reproduce:
> > > - enable autoindex in a directory
> > > - create file with name "some<em>thing" in the directory
> > > - view the (broken) directory list in web
> > >
> > 
> > it should use ngx_escape_html - I've tried modifying it but I don't
> > know enough C to correctly fix it.
> 
> I'm currently looking in it, stay tuned.

Patch.

Maxim Dounin
-------------- next part --------------
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1262268599 -10800
# Node ID 5ad805f474d01becd42f1aa6ae9d802ce30bf9de
# Parent  e06f16583f28f61683e123360902b934fdb8f929
Autoindex: escape html in file names.

diff --git a/src/http/modules/ngx_http_autoindex_module.c b/src/http/modules/ngx_http_autoindex_module.c
--- a/src/http/modules/ngx_http_autoindex_module.c
+++ b/src/http/modules/ngx_http_autoindex_module.c
@@ -26,6 +26,7 @@ typedef struct {
     ngx_str_t      name;
     size_t         utf_len;
     size_t         escape;
+    size_t         escape_html;
 
     unsigned       dir:1;
     unsigned       colon:1;
@@ -138,7 +139,7 @@ ngx_http_autoindex_handler(ngx_http_requ
 {
     u_char                         *last, *filename, scale;
     off_t                           length;
-    size_t                          len, utf_len, allocated, root;
+    size_t                          len, char_len, escape_html, allocated, root;
     ngx_tm_t                        tm;
     ngx_err_t                       err;
     ngx_buf_t                      *b;
@@ -345,6 +346,9 @@ ngx_http_autoindex_handler(ngx_http_requ
         entry->escape = 2 * ngx_escape_uri(NULL, ngx_de_name(&dir), len,
                                            NGX_ESCAPE_HTML);
 
+        entry->escape_html = ngx_escape_html(NULL, entry->name.data,
+                                             entry->name.len);
+
         if (utf8) {
             entry->utf_len = ngx_utf8_length(entry->name.data, entry->name.len);
         } else {
@@ -363,10 +367,12 @@ ngx_http_autoindex_handler(ngx_http_requ
                       ngx_close_dir_n " \"%s\" failed", &path);
     }
 
+    escape_html = ngx_escape_html(NULL, r->uri.data, r->uri.len);
+
     len = sizeof(title) - 1
-          + r->uri.len
+          + r->uri.len + escape_html
           + sizeof(header) - 1
-          + r->uri.len
+          + r->uri.len + escape_html
           + sizeof("</h1>") - 1
           + sizeof("<hr><pre><a href=\"../\">../</a>" CRLF) - 1
           + sizeof("</pre><hr>") - 1
@@ -379,6 +385,7 @@ ngx_http_autoindex_handler(ngx_http_requ
             + 1                                          /* 1 is for "/" */
             + sizeof("\">") - 1
             + entry[i].name.len - entry[i].utf_len + entry[i].colon * 2
+            + entry[i].escape_html
             + NGX_HTTP_AUTOINDEX_NAME_LEN + sizeof("&gt;") - 2
             + sizeof("</a>") - 1
             + sizeof(" 28-Sep-1970 12:00 ") - 1
@@ -398,9 +405,18 @@ ngx_http_autoindex_handler(ngx_http_requ
     }
 
     b->last = ngx_cpymem(b->last, title, sizeof(title) - 1);
-    b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
-    b->last = ngx_cpymem(b->last, header, sizeof(header) - 1);
-    b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
+
+    if (escape_html) {
+        b->last = (u_char *) ngx_escape_html(b->last, r->uri.data, r->uri.len);
+        b->last = ngx_cpymem(b->last, header, sizeof(header) - 1);
+        b->last = (u_char *) ngx_escape_html(b->last, r->uri.data, r->uri.len);
+
+    } else {
+        b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
+        b->last = ngx_cpymem(b->last, header, sizeof(header) - 1);
+        b->last = ngx_cpymem(b->last, r->uri.data, r->uri.len);
+    }
+
     b->last = ngx_cpymem(b->last, "</h1>", sizeof("</h1>") - 1);
 
     b->last = ngx_cpymem(b->last, "<hr><pre><a href=\"../\">../</a>" CRLF,
@@ -438,20 +454,41 @@ ngx_http_autoindex_handler(ngx_http_requ
 
         if (entry[i].name.len != len) {
             if (len > NGX_HTTP_AUTOINDEX_NAME_LEN) {
-                utf_len = NGX_HTTP_AUTOINDEX_NAME_LEN - 3 + 1;
+                char_len = NGX_HTTP_AUTOINDEX_NAME_LEN - 3 + 1;
 
             } else {
-                utf_len = NGX_HTTP_AUTOINDEX_NAME_LEN + 1;
+                char_len = NGX_HTTP_AUTOINDEX_NAME_LEN + 1;
             }
 
+            last = b->last;
             b->last = ngx_utf8_cpystrn(b->last, entry[i].name.data,
-                                       utf_len, entry[i].name.len + 1);
+                                       char_len, entry[i].name.len + 1);
+
+            if (entry[i].escape_html) {
+                b->last = (u_char *) ngx_escape_html(last, entry[i].name.data,
+                                                     b->last - last);
+            }
+
             last = b->last;
 
         } else {
-            b->last = ngx_cpystrn(b->last, entry[i].name.data,
-                                  NGX_HTTP_AUTOINDEX_NAME_LEN + 1);
-            last = b->last - 3;
+            if (entry[i].escape_html) {
+                if (len > NGX_HTTP_AUTOINDEX_NAME_LEN) {
+                    char_len = NGX_HTTP_AUTOINDEX_NAME_LEN - 3;
+
+                } else {
+                    char_len = len;
+                }
+
+                b->last = (u_char *) ngx_escape_html(b->last,
+                                                  entry[i].name.data, char_len);
+                last = b->last;
+
+            } else {
+                b->last = ngx_cpystrn(b->last, entry[i].name.data,
+                                      NGX_HTTP_AUTOINDEX_NAME_LEN + 1);
+                last = b->last - 3;
+            }
         }
 
         if (len > NGX_HTTP_AUTOINDEX_NAME_LEN) {