[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 <p.wassi at gmx.at>
Signed-off-by: Jo-Philipp Wich <jo at mein.io>
---
 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",
 				&path_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 &p;
@@ -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