[LEDE-DEV] [PATH] uhttpd: store relative query string offset

[Jo-Philipp Wich]

Instead of storing a pointer to the beginning of the query string within the
request url, store the byte offset instead.

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 
Signed-off-by: Jo-Philipp Wich 
---
 file.c   | 13 +++--
 lua.c|  2 +-
 proc.c   |  2 +-
 uhttpd.h |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/file.c b/file.c
index a1775f5..5ca8db5 100644
--- a/file.c
+++ b/file.c
@@ -156,7 +156,7 @@ uh_path_lookup(struct client *cl, const char *url)
 
/* separate query string from url */
if ((pathptr = strchr(url, '?')) != NULL) {
-   p.query = pathptr[1] ? pathptr + 1 : NULL;
+   p.query_offset = pathptr[1] ? (pathptr - url + 1) : 0;
 
/* urldecode component w/o query */
if (pathptr > url) {
@@ -239,8 +239,8 @@ uh_path_lookup(struct client *cl, const char *url)
ustream_printf(cl->us, "Content-Length: 0\r\n");
ustream_printf(cl->us, "Location: %s%s%s\r\n\r\n",
_phys[docroot_len],
-   p.query ? "?" : "",
-   p.query ? p.query : "");
+   p.query_offset ? "?" : "",
+   p.query_offset ? (url + p.query_offset) : "");
uh_request_done(cl);
p.redirected = 1;
return 
@@ -733,14 +733,13 @@ static int field_len(const char *ptr)
_field(root) \
_field(phys) \
_field(name) \
-   _field(info) \
-   _field(query)
+   _field(info)
 
 static void
 uh_defer_script(struct client *cl, struct dispatch_handler *d, struct 
path_info *pi)
 {
struct deferred_request *dr;
-   char *_root, *_phys, *_name, *_info, *_query;
+   char *_root, *_phys, *_name, *_info;
 
cl->dispatch.req_free = uh_free_pending_request;
 
@@ -757,6 +756,8 @@ uh_defer_script(struct client *cl, struct dispatch_handler 
*d, struct path_info
 #undef _field
 #define _field(_name) if (pi->_name) dr->pi._name = strcpy(_##_name, 
pi->_name);
path_info_fields
+
+   dr->pi.query_offset = pi->query_offset;
} else {
dr = calloc(1, sizeof(*dr));
}
diff --git a/lua.c b/lua.c
index 3efe22b..2ad8d35 100644
--- a/lua.c
+++ b/lua.c
@@ -219,7 +219,7 @@ static void lua_main(struct client *cl, struct path_info 
*pi, char *url)
str = strchr(url, '?');
if (str) {
if (*(str + 1))
-   pi->query = str + 1;
+   pi->query_offset = str - url + 1;
path_len = str - url;
}
 
diff --git a/proc.c b/proc.c
index e360897..ca22430 100644
--- a/proc.c
+++ b/proc.c
@@ -145,7 +145,7 @@ struct env_var *uh_get_process_vars(struct client *cl, 
struct path_info *pi)
extra_vars[VAR_SCRIPT_NAME].value = pi->name;
extra_vars[VAR_SCRIPT_FILE].value = pi->phys;
extra_vars[VAR_DOCROOT].value = pi->root;
-   extra_vars[VAR_QUERY].value = pi->query ? pi->query : "";
+   extra_vars[VAR_QUERY].value = pi->query_offset ? (url + 
pi->query_offset) : "";
extra_vars[VAR_REQUEST].value = url;
extra_vars[VAR_PROTO].value = http_versions[req->version];
extra_vars[VAR_METHOD].value = http_methods[req->method];
diff --git a/uhttpd.h b/uhttpd.h
index 374cd72..6e77457 100644
--- a/uhttpd.h
+++ b/uhttpd.h
@@ -145,7 +145,7 @@ struct path_info {
const char *phys;
const char *name;
const char *info;
-   const char *query;
+   size_t query_offset;
bool redirected;
struct stat stat;
const struct interpreter *ip;
-- 
2.14.2


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATH] uhttpd: store relative query string offset

[Jo-Philipp Wich]

Hi,

please give this patch a try. I was able to reproduce your issue and
this patch fixed the issue for me.

~ Jo

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev