file: fix query string handling

Instead of storing a pointer to the beginning of the query string within the
request url, store a copy in a static buffer instead. This aligns handling
the query string portion of the url with other elements like physical path
or path info information.

Since the URL is usually kept in the per-client blob buffer which might
change its memory location due to reallocations triggered by blobmsg_add_*,
it is not safe to point to it early in the request life cycle.

This fixes invalid memory access usually manifesting itself as corrupted
query string data in CGI scripts.

Reported-by: P. Wassi <p.wassi@gmx.at>
Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/file.c b/file.c
index a1775f5..b3702c8 100644 (file)
--- a/file.c
+++ b/file.c
@@ -132,6 +132,7 @@ uh_path_lookup(struct client *cl, const char *url)
 {
        static char path_phys[PATH_MAX];
        static char path_info[PATH_MAX];
 {
        static char path_phys[PATH_MAX];
        static char path_info[PATH_MAX];

+       static char path_query[PATH_MAX];

        static struct path_info p;
 
        const char *docroot = conf.docroot;
        static struct path_info p;
 
        const char *docroot = conf.docroot;


@@ -156,7 +157,11 @@ uh_path_lookup(struct client *cl, const char *url)
 
        /* separate query string from url */
        if ((pathptr = strchr(url, '?')) != NULL) {
 
        /* separate query string from url */
        if ((pathptr = strchr(url, '?')) != NULL) {

-               p.query = pathptr[1] ? pathptr + 1 : NULL;
+               if (pathptr[1]) {
+                       p.query = path_query;
+                       snprintf(path_query, sizeof(path_query), "%s",
+                                pathptr + 1);
+               }

 
                /* urldecode component w/o query */
                if (pathptr > url) {
 
                /* urldecode component w/o query */
                if (pathptr > url) {