Problems when using ssi includes with subdirectories.

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 */

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 */

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.

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.

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)

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.

I added a urldecode before the path simplify. Otherwise the attached patch was applied in [1021]

vcbvc