The memory pointed to by g_query gets overwritten when the index_page
is used, causing URL arguments to get dropped when we fall back to
/cgi-bin/index.cgi. QUERY_STRING if furthermore hijacked to pass the
original (<DIR>/) URI to the CGI script, which is quite non-standard
and disallows use of URL arguments.

Fix it by instead passing the original URI in REQUEST_URI, and make a
deep copy of the URL arguments before they get overwritten, if needed.

Also update httpd_indexcgi to take the directory location from
REQUEST_URI instead.

function                                             old     new   delta
handle_incoming_and_exit                            2814    2829     +15
send_cgi_and_exit                                    983     986      +3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 18/0)               Total: 18 bytes

Signed-off-by: Peter Korsgaard <[email protected]>
---
 networking/httpd.c          |   27 +++++++++++++++++++--------
 networking/httpd_indexcgi.c |   25 +++++++++++++++----------
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/networking/httpd.c b/networking/httpd.c
index e9cd213..b500307 100644
--- a/networking/httpd.c
+++ b/networking/httpd.c
@@ -1265,18 +1265,21 @@ static void setenv1(const char *name, const char *value)
  *
  * Parameters:
  * const char *url              The requested URL (with leading /).
+ * const char *orig_uri         The original URI before rewriting (if any)
  * int post_len                 Length of the POST body.
  * const char *cookie           For set HTTP_COOKIE.
  * const char *content_type     For set CONTENT_TYPE.
  */
 static void send_cgi_and_exit(
                const char *url,
+               const char *orig_uri,
                const char *request,
                int post_len,
                const char *cookie,
                const char *content_type) NORETURN;
 static void send_cgi_and_exit(
                const char *url,
+               const char *orig_uri,
                const char *request,
                int post_len,
                const char *cookie,
@@ -1314,9 +1317,9 @@ static void send_cgi_and_exit(
        setenv1("PATH_INFO", script);   /* set to /PATH_INFO or "" */
        setenv1("REQUEST_METHOD", request);
        if (g_query) {
-               putenv(xasprintf("%s=%s?%s", "REQUEST_URI", url, g_query));
+               putenv(xasprintf("%s=%s?%s", "REQUEST_URI", orig_uri, g_query));
        } else {
-               setenv1("REQUEST_URI", url);
+               setenv1("REQUEST_URI", orig_uri);
        }
        if (script != NULL)
                *script = '\0';         /* cut off /PATH_INFO */
@@ -2248,12 +2251,21 @@ static void handle_incoming_and_exit(const 
len_and_sockaddr *fromAddr)
                        /* protect listing "cgi-bin/" */
                        send_headers_and_exit(HTTP_FORBIDDEN);
                }
-               send_cgi_and_exit(urlcopy, prequest, length, cookie, 
content_type);
+               send_cgi_and_exit(urlcopy, urlcopy, prequest, length, cookie, 
content_type);
        }
 #endif
 
-       if (urlp[-1] == '/')
+       if (urlp[-1] == '/') {
+               /* when index_page string is appended to <dir>/ URL, it 
overwrites
+                  the query string. If we fallback to call /cgi-bin/index.cgi,
+                  query string would be lost and not available to the CGI.
+                  Work around it by making a deep copy instead.
+               */
+               if (ENABLE_FEATURE_HTTPD_CGI)
+                       g_query = xstrdup(g_query);
+
                strcpy(urlp, index_page);
+       }
        if (stat(tptr, &sb) == 0) {
 #if ENABLE_FEATURE_HTTPD_CONFIG_WITH_SCRIPT_INTERPR
                char *suffix = strrchr(tptr, '.');
@@ -2261,7 +2273,7 @@ static void handle_incoming_and_exit(const 
len_and_sockaddr *fromAddr)
                        Htaccess *cur;
                        for (cur = script_i; cur; cur = cur->next) {
                                if (strcmp(cur->before_colon + 1, suffix) == 0) 
{
-                                       send_cgi_and_exit(urlcopy, prequest, 
length, cookie, content_type);
+                                       send_cgi_and_exit(urlcopy, urlcopy, 
prequest, length, cookie, content_type);
                                }
                        }
                }
@@ -2274,9 +2286,8 @@ static void handle_incoming_and_exit(const 
len_and_sockaddr *fromAddr)
                /* It's a dir URL and there is no index.html
                 * Try cgi-bin/index.cgi */
                if (access("/cgi-bin/index.cgi"+1, X_OK) == 0) {
-                       urlp[0] = '\0';
-                       g_query = urlcopy;
-                       send_cgi_and_exit("/cgi-bin/index.cgi", prequest, 
length, cookie, content_type);
+                       urlp[0] = '\0'; /* remove index_page */
+                       send_cgi_and_exit("/cgi-bin/index.cgi", urlcopy, 
prequest, length, cookie, content_type);
                }
        }
        /* else fall through to send_file, it errors out if open fails: */
diff --git a/networking/httpd_indexcgi.c b/networking/httpd_indexcgi.c
index 7e0225e..2c73ff7 100644
--- a/networking/httpd_indexcgi.c
+++ b/networking/httpd_indexcgi.c
@@ -221,20 +221,25 @@ int main(int argc, char *argv[])
        unsigned long long size_total;
        int odd;
        DIR *dirp;
-       char *QUERY_STRING;
-
-       QUERY_STRING = getenv("QUERY_STRING");
-       if (!QUERY_STRING
-        || QUERY_STRING[0] != '/'
-        || strstr(QUERY_STRING, "//")
-        || strstr(QUERY_STRING, "/../")
-        || strcmp(strrchr(QUERY_STRING, '/'), "/..") == 0
+       char *location, *t;
+
+       location = getenv("REQUEST_URI");
+       if (!location
+        || location[0] != '/'
+        || strstr(location, "//")
+        || strstr(location, "/../")
+        || strcmp(strrchr(location, '/'), "/..") == 0
        ) {
                return 1;
        }
 
+       /* drop URL arguments if any */
+       t = strchr(location, '?');
+       if (t)
+               *t = '\0';
+
        if (chdir("..")
-        || (QUERY_STRING[1] && chdir(QUERY_STRING + 1))
+        || (location[1] && chdir(location + 1))
        ) {
                return 1;
        }
@@ -271,14 +276,14 @@ int main(int argc, char *argv[])
                "\r\n" /* Mandatory empty line after headers */
                "<html><head><title>Index of ");
        /* Guard against directories with &, > etc */
-       fmt_html(QUERY_STRING);
+       fmt_html(location);
        fmt_str(
                "</title>\n"
                STYLE_STR
                "</head>" "\n"
                "<body>" "\n"
                "<h1>Index of ");
-       fmt_html(QUERY_STRING);
+       fmt_html(location);
        fmt_str(
                "</h1>" "\n"
                "<table>" "\n"
-- 
1.7.7.1

_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to