When encoding the Location url, only encode the query and path

elements from the user input and not the constants from the
configuration.  This makes it possible to specify chars like '?' in
the uri.

OK Sebastien Marie

1 files changed, 37 insertions, 22 deletions

diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
index 99d52674790..472fe728100 100644
--- a/usr.sbin/httpd/server_http.c
+++ b/usr.sbin/httpd/server_http.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_http.c,v 1.80 2015/05/20 09:28:47 kettenis Exp $	*/
+/*	$OpenBSD: server_http.c,v 1.81 2015/06/21 13:08:36 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -738,7 +738,7 @@ server_abort_http(struct client *clt, u_int code, const char *msg)
 	const char		*httperr = NULL, *style;
 	char			*httpmsg, *body = NULL, *extraheader = NULL;
 	char			 tmbuf[32], hbuf[128];
-	char			 buf[IBUF_READ_SIZE], *ptr = NULL;
+	char			 buf[IBUF_READ_SIZE];
 	int			 bodylen;
 
 	if (code == 0) {
@@ -770,16 +770,13 @@ server_abort_http(struct client *clt, u_int code, const char *msg)
 		if (msg == NULL)
 			break;
 		memset(buf, 0, sizeof(buf));
-		if ((ptr = server_expand_http(clt, msg,
-		    buf, sizeof(buf))) == NULL)
+		if (server_expand_http(clt, msg, buf, sizeof(buf)) == NULL)
 			goto done;
-		if ((ptr = url_encode(ptr)) == NULL)
-			goto done;
-		if (asprintf(&extraheader, "Location: %s\r\n", ptr) == -1) {
+		if (asprintf(&extraheader, "Location: %s\r\n", buf) == -1) {
 			code = 500;
 			extraheader = NULL;
 		}
-		msg = ptr;
+		msg = buf;
 		break;
 	case 401:
 		if (asprintf(&extraheader,
@@ -858,7 +855,6 @@ server_abort_http(struct client *clt, u_int code, const char *msg)
 		server_close(clt, httpmsg);
 		free(httpmsg);
 	}
-	free(ptr);
 }
 
 void
@@ -885,20 +881,30 @@ server_expand_http(struct client *clt, const char *val, char *buf,
 {
 	struct http_descriptor	*desc = clt->clt_descreq;
 	struct server_config	*srv_conf = clt->clt_srv_conf;
-	char			 ibuf[128], *str;
+	char			 ibuf[128], *str, *path, *query;
+	int			 ret;
 
 	if (strlcpy(buf, val, len) >= len)
 		return (NULL);
 
 	if (strstr(val, "$DOCUMENT_URI") != NULL) {
-		if (expand_string(buf, len, "$DOCUMENT_URI",
-		    desc->http_path) != 0)
+		if ((path = url_encode(desc->http_path)) == NULL)
+			return (NULL);
+		ret = expand_string(buf, len, "$DOCUMENT_URI", path);
+		free(path);
+		if (ret != 0)
 			return (NULL);
 	}
 	if (strstr(val, "$QUERY_STRING") != NULL) {
-		if (expand_string(buf, len, "$QUERY_STRING",
-		    desc->http_query == NULL ? "" :
-		    desc->http_query) != 0)
+		if (desc->http_query == NULL) {
+			ret = expand_string(buf, len, "$QUERY_STRING", "");
+		} else {
+			if ((query = url_encode(desc->http_query)) == NULL)
+				return (NULL);
+			ret = expand_string(buf, len, "$QUERY_STRING", query);
+			free(query);
+		}
+		if (ret != 0)
 			return (NULL);
 	}
 	if (strstr(val, "$REMOTE_") != NULL) {
@@ -929,17 +935,26 @@ server_expand_http(struct client *clt, const char *val, char *buf,
 		}
 	}
 	if (strstr(val, "$REQUEST_URI") != NULL) {
+		if ((path = url_encode(desc->http_path)) == NULL)
+			return (NULL);
 		if (desc->http_query == NULL) {
-			if ((str = strdup(desc->http_path)) == NULL)
+			str = path;
+		} else {
+			if ((query = url_encode(desc->http_query)) == NULL) {
+				free(path);
+				return (NULL);
+			}
+			ret = asprintf(&str, "%s?%s", path, query);
+			free(path);
+			free(query);
+			if (ret == -1)
 				return (NULL);
-		} else if (asprintf(&str, "%s?%s",
-		    desc->http_path, desc->http_query) == -1)
-			return (NULL);
-		if (expand_string(buf, len, "$REQUEST_URI", str) != 0) {
-			free(str);
-			return (NULL);
 		}
+
+		ret = expand_string(buf, len, "$REQUEST_URI", str);
 		free(str);
+		if (ret != 0)
+			return (NULL);
 	}
 	if (strstr(val, "$SERVER_") != NULL) {
 		if (strstr(val, "$SERVER_ADDR") != NULL) {