Project

General

Profile

Actions

Bug #462

closed

Problems when using ssi includes with subdirectories.

Added by robe over 16 years ago. Updated about 15 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_ssi
Target version:
-
ASK QUESTIONS IN Forums:

Description


mirror:/var/www# cat test.shtml
<!--#include file="ssitest/ssi.txt" -->
mirror:/var/www# cat ssitest/ssi.txt
foo
mirror:/var/www#

yields the following error:

2006-01-12 13:21:12: (mod_ssi.c.583) ssi: stating failed /var/www/ssi.txt No such file or directory


Files

lighttpd-mod-ssi.patch (1.06 KB) lighttpd-mod-ssi.patch robe, 2006-02-20 10:53
Actions #1

Updated by conny over 16 years ago

From #521:
----
The following patch should fix the SSI_INCLUDE for #include file="" when using sub-directory:


--- mod_ssi.c.dist      2006-02-10 13:33:00.000000000 -0500
+++ mod_ssi.c.new       2006-02-13 15:23:00.000000000 -0500
@@ -513,18 +513,18 @@

                if (file_path) {
                        /* current doc-root */
-                       if (NULL == (sl = strrchr(con->physical.path->ptr, '/'))) {
-                               buffer_copy_string(p->stat_fn, "/");
-                       } else {
-                               buffer_copy_string_len(p->stat_fn, con->physical.path->ptr, sl - con->physical.path->ptr + 1);
-                       }
-
-                       /* fn */
-                       if (NULL == (sl = strrchr(file_path, '/'))) {
-                               buffer_append_string(p->stat_fn, file_path);
-                       } else {
-                               buffer_append_string(p->stat_fn, sl + 1);
-                       }
+            //
+            // skip if file_path contains forbidden strings
+            if (file_path[0] == '/' || strstr(file_path, "../")) break;
+
+            if (NULL == (sl = strrchr(con->physical.path->ptr, '/'))) {
+                buffer_copy_string(p->stat_fn, "/");
+            } else {
+                buffer_copy_string_len(p->stat_fn, con->physical.path->ptr, sl - con->physical.path->ptr + 1);
+            }
+
+            buffer_append_string(p->stat_fn, file_path);
+
                } else {
                        /* virtual */
Actions #2

Updated by conny over 16 years ago

From #526:
----
The following patch solves the sub-directory problem in #include file="argument" and makes sure no forbidden strings are part of the argument:


--- mod_ssi.c.dist      2006-02-10 13:33:00.000000000 -0500
+++ mod_ssi.c  2006-02-15 12:38:00.000000000 -0500
@@ -513,18 +513,17 @@

                if (file_path) {
                        /* current doc-root */
+
+                       // skip if file_path contains forbidden strings
+                       if (file_path[0] == '/' || strstr(file_path, "../")) break;
+
                        if (NULL == (sl = strrchr(con->physical.path->ptr, '/'))) {
                                buffer_copy_string(p->stat_fn, "/");
                        } else {
                                buffer_copy_string_len(p->stat_fn, con->physical.path->ptr, sl - con->physical.path->ptr + 1);
                        }

-                       /* fn */
-                       if (NULL == (sl = strrchr(file_path, '/'))) {
-                               buffer_append_string(p->stat_fn, file_path);
-                       } else {
-                               buffer_append_string(p->stat_fn, sl + 1);
-                       }
+                       buffer_append_string(p->stat_fn, file_path);
                } else {
                        /* virtual */

Actions #3

Updated by robe over 16 years ago

I already sent jan a patch 2 weeks ago, apparently he's been too busy to apply it.

buffer_path_simplify does everything we want, so we don't have to play around sanitizing the path ourselves.

Actions #4

Updated by Anonymous over 16 years ago

Using buffer_path_simplify() handles this, BUT a "../" or "/" is NOT ALLOWED in #include file="" according to the specs... this would allow one to go OUTSIDE of the document root.

The patch I provided made sure the include was REFUSED if those patterns were found in the file="" argument. (see below):


    // skip if file_path contains forbidden strings
    if (file_path[0] == '/' || strstr(file_path, "../")) break;

Thanks.

-- r4l

Actions #5

Updated by robe over 16 years ago

No, it doesn't allow you to go outside of the documentroot since buffer_path_simplify strips excess path members (.., /, etc) and the sanitized path gets then appended to p->stat_fn, which contains the current directory.

But yeah, refusing it may be the correct way (although I've yet to see a "specification", all I've found till now is the documentation of NCSA httpd and apache on the topic of SSI), although it's currently a bit suboptimal since mod_ssi lacks error reporting to the enduser. (That's how apache does it: http://amd.co.at/test.shtml)

Actions #6

Updated by Anonymous over 16 years ago

One way I found to "report" to the user is to replace the SSI_INCLUDE code with an HTML comment that gives detail on the error... this is not visible when looking at the website, only when looking at the HTML source code.

Something along the line of:


buffer_copy_string(srv->tmp_buf, "<!-- error in ssi directive -->");
chunkqueue_append_buffer(con->write_queue, srv->tmp_buf);
break;

Thanks.

-- marc

Actions #7

Updated by jan over 16 years ago

  • Status changed from New to Fixed
  • Resolution set to fixed

I added a urldecode before the path simplify. Otherwise the attached patch was applied in r1021

Actions #8

Updated by Anonymous about 16 years ago

vcbvc

Actions

Also available in: Atom