From: reyk Date: Wed, 23 Jul 2014 21:43:12 +0000 (+0000) Subject: First attempt at verifying the request path and the access X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5d9e55e4ba46f45d36c3974a2c023f70a328046b;p=openbsd First attempt at verifying the request path and the access permissions. We also have to redirect with 301 if a directory name was requested without the trailing slash. --- diff --git a/usr.sbin/httpd/server_file.c b/usr.sbin/httpd/server_file.c index e23086fd93d..8a818db84de 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.7 2014/07/23 19:03:56 reyk Exp $ */ +/* $OpenBSD: server_file.c,v 1.8 2014/07/23 21:43:12 reyk Exp $ */ /* * Copyright (c) 2006 - 2014 Reyk Floeter @@ -46,8 +46,71 @@ #include "httpd.h" #include "http.h" +int server_file_access(struct http_descriptor *, char *, size_t, + struct stat *); void server_file_error(struct bufferevent *, short, void *); +int +server_file_access(struct http_descriptor *desc, char *path, size_t len, + struct stat *st) +{ + errno = 0; + if (access(path, R_OK) == -1) { + goto fail; + } else if (stat(path, st) == -1) { + goto fail; + } else if (S_ISDIR(st->st_mode)) { + /* XXX Should we support directory listing? */ + + if (!len) { + /* Recursion - the index "file" is a directory? */ + errno = EACCES; + goto fail; + } + + /* 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; + } + + /* Indicate that the file has been moved */ + return (301); + } + + /* Otherwise append the default index file */ + if (strlcat(path, HTTPD_INDEX, len) >= len) { + errno = EACCES; + goto fail; + } + + /* Check again but set len to 0 to avoid recursion */ + 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; + goto fail; + } + + return (0); + + fail: + switch (errno) { + case ENOENT: + return (404); + case EACCES: + return (403); + default: + return (500); + } + + /* NOTREACHED */ +} + int server_file(struct httpd *env, struct client *clt) { @@ -59,41 +122,21 @@ server_file(struct httpd *env, struct client *clt) char path[MAXPATHLEN]; struct stat st; - /* - * XXX This is not ready XXX - * XXX Don't expect anything from this code yet, - */ - if (canonicalize_path(HTTPD_DOCROOT, desc->http_path, path, sizeof(path)) == NULL) { - server_abort_http(clt, 404, path); + /* Do not echo the uncanonicalized path */ + server_abort_http(clt, 500, "invalid request path"); return (-1); } - /* Prepend default index file */ - if (path[strlen(path) - 1] == '/' && - strlcat(path, HTTPD_INDEX, sizeof(path)) >= sizeof(path)) { - server_abort_http(clt, 404, path); - return (-1); - } - - if (access(path, R_OK) == -1) { - strlcpy(path, desc->http_path, sizeof(path)); - switch (errno) { - case EACCES: - server_abort_http(clt, 403, path); - break; - case ENOENT: - server_abort_http(clt, 404, path); - break; - default: - server_abort_http(clt, 500, path); - break; - } + /* Returns HTTP status code on error */ + if ((ret = server_file_access(desc, path, sizeof(path), &st)) != 0) { + server_abort_http(clt, ret, path); return (-1); } - if ((fd = open(path, O_RDONLY)) == -1 || fstat(fd, &st) == -1) + /* Now open the file, should be readable or we have another problem */ + if ((fd = open(path, O_RDONLY)) == -1) goto fail; /* File descriptor is opened, decrement inflight counter */ diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c index 07d042a80fd..6e154563e43 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.9 2014/07/17 11:35:26 stsp Exp $ */ +/* $OpenBSD: server_http.c,v 1.10 2014/07/23 21:43:12 reyk Exp $ */ /* * Copyright (c) 2006 - 2014 Reyk Floeter @@ -520,7 +520,7 @@ server_abort_http(struct client *clt, u_int code, const char *msg) struct server *srv = clt->clt_server; struct bufferevent *bev = clt->clt_bev; const char *httperr = NULL, *text = ""; - char *httpmsg; + char *httpmsg, *extraheader = NULL; time_t t; struct tm *lt; char tmbuf[32], hbuf[128]; @@ -544,8 +544,21 @@ server_abort_http(struct client *clt, u_int code, const char *msg) tmbuf[strlen(tmbuf) - 1] = '\0'; /* skip final '\n' */ /* Do not send details of the Internal Server Error */ - if (code != 500) + switch (code) { + case 500: + /* Do not send details of the Internal Server Error */ + break; + case 301: + case 302: + if (asprintf(&extraheader, "Location: %s\r\n", msg) == -1) { + code = 500; + extraheader = NULL; + } + break; + default: text = msg; + break; + } /* A CSS stylesheet allows minimal customization by the user */ style = "body { background-color: white; color: black; font-family: " @@ -564,6 +577,7 @@ server_abort_http(struct client *clt, u_int code, const char *msg) "Server: %s\r\n" "Connection: close\r\n" "Content-Type: text/html\r\n" + "%s" "\r\n" "\n" @@ -579,6 +593,7 @@ server_abort_http(struct client *clt, u_int code, const char *msg) "\n" "\n", code, httperr, tmbuf, HTTPD_SERVERNAME, + extraheader == NULL ? "" : extraheader, code, httperr, style, httperr, text, HTTPD_SERVERNAME, hbuf, ntohs(srv->srv_conf.port)) == -1) goto done; @@ -588,6 +603,7 @@ server_abort_http(struct client *clt, u_int code, const char *msg) free(httpmsg); done: + free(extraheader); if (asprintf(&httpmsg, "%s (%03d %s)", msg, code, httperr) == -1) server_close(clt, msg); else {