Canonicalize the request path once without the docroot and prepend the

docroot only only when it's needed.  Suggested by deraadt@.

4 files changed, 25 insertions, 33 deletions

diff --git a/usr.sbin/httpd/httpd.c b/usr.sbin/httpd/httpd.c
index 22ae5f9a445..454814d6470 100644
--- a/usr.sbin/httpd/httpd.c
+++ b/usr.sbin/httpd/httpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.c,v 1.8 2014/07/25 16:23:19 reyk Exp $	*/
+/*	$OpenBSD: httpd.c,v 1.9 2014/07/25 21:29:58 reyk Exp $	*/
 
 /*
  * Copyright (c) 2014 Reyk Floeter <reyk@openbsd.org>
@@ -466,24 +466,15 @@ canonicalize_host(const char *host, char *name, size_t len)
 }
 
 const char *
-canonicalize_path(const char *root, const char *input, char *path, size_t len)
+canonicalize_path(const char *input, char *path, size_t len)
 {
 	const char	*i;
 	char		*p, *start, *end;
-	size_t		 n;
 
 	/* assuming input starts with '/' and is nul-terminated */
 	i = input;
 	p = path;
 
-	/* prepend root directory, if specified */
-	if (root != NULL) {
-		if ((n = strlcpy(path, root, len)) >= len)
-			return (NULL);
-		len -= n;
-		p += n;
-	}
-
 	if (*input != '/' || len < 3)
 		return (NULL);
 
diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index cc10bfb53f4..f2e3dcec5ef 100644
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.h,v 1.12 2014/07/25 16:23:19 reyk Exp $	*/
+/*	$OpenBSD: httpd.h,v 1.13 2014/07/25 21:29:58 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -421,7 +421,7 @@ void		 event_again(struct event *, int, short,
 		    void (*)(int, short, void *),
 		    struct timeval *, struct timeval *, void *);
 const char	*canonicalize_host(const char *, char *, size_t);
-const char	*canonicalize_path(const char *, const char *, char *, size_t);
+const char	*canonicalize_path(const char *, char *, size_t);
 void		 imsg_event_add(struct imsgev *);
 int		 imsg_compose_event(struct imsgev *, u_int16_t, u_int32_t,
 		    pid_t, int, void *, u_int16_t);
diff --git a/usr.sbin/httpd/server_file.c b/usr.sbin/httpd/server_file.c
index 5a360296558..81346ce3159 100644
--- a/usr.sbin/httpd/server_file.c
+++ b/usr.sbin/httpd/server_file.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_file.c,v 1.14 2014/07/25 20:13:06 reyk Exp $	*/
+/*	$OpenBSD: server_file.c,v 1.15 2014/07/25 21:29:58 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -54,7 +54,9 @@ int
 server_file_access(struct http_descriptor *desc, char *path, size_t len,
     struct stat *st)
 {
+	char	*newpath;
 	errno = 0;
+
 	if (access(path, R_OK) == -1) {
 		goto fail;
 	} else if (stat(path, st) == -1) {
@@ -70,13 +72,10 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
 
 		/* Redirect to path with trailing "/" */
 		if (path[strlen(path) - 1] != '/') {
-			/* Remove the document root to get the relative URL */
-			if (canonicalize_path(NULL,
-			    desc->http_path, path, len) == NULL ||
-			    strlcat(path, "/", len) >= len) {
-				errno = EINVAL;
-				goto fail;
-			}
+			if (asprintf(&newpath, "%s/", desc->http_path) == -1)
+				return (500);
+			free(desc->http_path);
+			desc->http_path = newpath;
 
 			/* Indicate that the file has been moved */
 			return (301);
@@ -89,8 +88,7 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
 		}
 
 		/* Check again but set len to 0 to avoid recursion */
-		if (server_file_access(desc, path, 0, st) != 0)
-			goto fail;
+		return (server_file_access(desc, path, 0, st));
 	} else if (!S_ISREG(st->st_mode)) {
 		/* Don't follow symlinks and ignore special files */
 		errno = EACCES;
@@ -100,10 +98,6 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
 	return (0);
 
  fail:
-	/* Remove the document root */
-	if (len && canonicalize_path(NULL, desc->http_path, path, len) == NULL)
-		return (500);
-
 	switch (errno) {
 	case ENOENT:
 		return (404);
@@ -127,16 +121,17 @@ server_file(struct httpd *env, struct client *clt)
 	char			 path[MAXPATHLEN];
 	struct stat		 st;
 
-	if (canonicalize_path(srv_conf->docroot,
-	    desc->http_path, path, sizeof(path)) == NULL) {
+	/* Request path is already canonicalized */
+	if ((size_t)snprintf(path, sizeof(path), "%s/%s",
+	    srv_conf->docroot, desc->http_path) >= sizeof(path)) {
 		/* Do not echo the uncanonicalized path */
-		server_abort_http(clt, 500, "invalid request path");
+		server_abort_http(clt, 500, desc->http_path);
 		return (-1);
 	}
 
 	/* Returns HTTP status code on error */
 	if ((ret = server_file_access(desc, path, sizeof(path), &st)) != 0) {
-		server_abort_http(clt, ret, path);
+		server_abort_http(clt, ret, desc->http_path);
 		return (-1);
 	}
 
diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
index 98edb96f24c..97d3a599df4 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.14 2014/07/25 16:23:19 reyk Exp $	*/
+/*	$OpenBSD: server_http.c,v 1.15 2014/07/25 21:29:58 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -627,13 +627,19 @@ server_close_http(struct client *clt)
 int
 server_response(struct httpd *httpd, struct client *clt)
 {
+	char			 path[MAXPATHLEN];
 	struct http_descriptor	*desc	= clt->clt_desc;
 	struct server		*srv = clt->clt_srv;
 	struct server_config	*srv_conf;
 	struct kv		*kv, key;
 	int			 ret;
 
-	if (desc->http_path == NULL)
+	/* Canonicalize the request path */
+	if (desc->http_path == NULL ||
+	    canonicalize_path(desc->http_path, path, sizeof(path)) == NULL)
+		goto fail;
+	free(desc->http_path);
+	if ((desc->http_path = strdup(path)) == NULL)
 		goto fail;
 
 	if (strcmp(desc->http_version, "HTTP/1.1") == 0) {