From 2f1a836bf092f02159da45a81cfe93cdd8b34bfe Mon Sep 17 00:00:00 2001 From: robert Date: Fri, 30 Jun 2023 05:25:58 -0400 Subject: [PATCH] enhanced path sanitizing --- examples/file-upload-multiple-posts/main.c | 6 ++- mongoose.c | 42 +++++++++--------- mongoose.h | 50 +++++++++++----------- src/http.c | 19 ++++++-- src/str.c | 23 +++------- src/str.h | 2 +- test/unit_test.c | 23 +++++----- 7 files changed, 86 insertions(+), 79 deletions(-) diff --git a/examples/file-upload-multiple-posts/main.c b/examples/file-upload-multiple-posts/main.c index dd38249e..b1b8c4b9 100644 --- a/examples/file-upload-multiple-posts/main.c +++ b/examples/file-upload-multiple-posts/main.c @@ -12,11 +12,13 @@ static void cb(struct mg_connection *c, int ev, void *ev_data, void *fn_data) { if (mg_http_match_uri(hm, "/upload")) { char path[80], name[64]; mg_http_get_var(&hm->query, "name", name, sizeof(name)); + mg_snprintf(path, sizeof(path), "/tmp/%s", name); if (name[0] == '\0') { mg_http_reply(c, 400, "", "%s", "name required"); + } else if (!mg_path_is_sane(path)) { + mg_http_reply(c, 400, "", "%s", "invalid path"); } else { - mg_snprintf(path, sizeof(path), "/tmp/%s", name); - mg_http_upload(c, hm, &mg_fs_posix, mg_remove_double_dots(path), 99999); + mg_http_upload(c, hm, &mg_fs_posix, path, 99999); } } else { struct mg_http_serve_opts opts = {.root_dir = "web_root"}; diff --git a/mongoose.c b/mongoose.c index 81695f4a..7d80bd9b 100644 --- a/mongoose.c +++ b/mongoose.c @@ -1842,6 +1842,7 @@ struct printdirentrydata { const char *dir; }; +#if MG_ENABLE_DIRLIST static void printdirentry(const char *name, void *userdata) { struct printdirentrydata *d = (struct printdirentrydata *) userdata; struct mg_fs *fs = d->opts->fs == NULL ? &mg_fs_posix : d->opts->fs; @@ -1948,6 +1949,7 @@ static void listdir(struct mg_connection *c, struct mg_http_message *hm, memcpy(c->send.buf + off - 12, tmp, n); // Set content length c->is_resp = 0; // Mark response end } +#endif // Resolve requested file into `path` and return its fs->st() result static int uri_to_path2(struct mg_connection *c, struct mg_http_message *hm, @@ -1956,13 +1958,20 @@ static int uri_to_path2(struct mg_connection *c, struct mg_http_message *hm, int flags, tmp; // Append URI to the root_dir, and sanitize it size_t n = mg_snprintf(path, path_size, "%.*s", (int) dir.len, dir.ptr); - if (n > path_size) n = path_size; + if (n > path_size) { + mg_http_reply(c, 400, "", "Exceeded path size"); + return -1; + } path[path_size - 1] = '\0'; - if (n + 2 < path_size) path[n++] = '/', path[n] = '\0'; + // Terminate root dir with / + if (n + 2 < path_size && path[n-1] != '/') path[n++] = '/', path[n] = '\0'; mg_url_decode(hm->uri.ptr + url.len, hm->uri.len - url.len, path + n, path_size - n, 0); path[path_size - 1] = '\0'; // Double-check - mg_remove_double_dots(path); + if (!mg_path_is_sane(path)) { + mg_http_reply(c, 400, "", "Invalid path"); + return -1; + } n = strlen(path); while (n > 1 && path[n - 1] == '/') path[--n] = 0; // Trim trailing slashes flags = mg_vcmp(&hm->uri, "/") == 0 ? MG_FS_DIR : fs->st(path, NULL, NULL); @@ -2022,7 +2031,11 @@ void mg_http_serve_dir(struct mg_connection *c, struct mg_http_message *hm, if (flags < 0) { // Do nothing: the response has already been sent by uri_to_path() } else if (flags & MG_FS_DIR) { +#if MG_ENABLE_DIRLIST listdir(c, hm, opts, path); +#else + mg_http_reply(c, 403, "", "Forbidden\n"); +#endif } else if (flags && sp != NULL && mg_globmatch(sp, strlen(sp), path, strlen(path))) { mg_http_serve_ssi(c, opts->root_dir, path); @@ -5402,25 +5415,14 @@ void mg_unhex(const char *buf, size_t len, unsigned char *to) { } } -char *mg_remove_double_dots(char *s) { - char *saved = s, *p = s; - while (*s != '\0') { - *p++ = *s++; - if (s[-1] == '/' || s[-1] == '\\') { - while (s[0] != '\0') { - if (s[0] == '/' || s[0] == '\\') { - s++; - } else if (s[0] == '.' && s[1] == '.' && - (s[2] == '/' || s[2] == '\\')) { - s += 2; - } else { - break; - } - } +bool mg_path_is_sane(const char *path) { + const char *s = path; + for (; s[0] != '\0'; s++) { + if (s == path || s[0] == '/' || s[0] == '\\') { // Subdir? + if (s[1] == '.' && s[2] == '.') return false; // Starts with .. } } - *p = '\0'; - return saved; + return true; } #ifdef MG_ENABLE_LINES diff --git a/mongoose.h b/mongoose.h index afd272be..3e30a253 100644 --- a/mongoose.h +++ b/mongoose.h @@ -256,30 +256,30 @@ static inline int mg_mkdir(const char *path, mode_t mode) { #include int mkdir(const char *, mode_t); #endif - - -#if MG_ARCH == MG_ARCH_RTTHREAD - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#ifndef MG_IO_SIZE -#define MG_IO_SIZE 1460 -#endif - -#endif // MG_ARCH == MG_ARCH_RTTHREAD + + +#if MG_ARCH == MG_ARCH_RTTHREAD + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef MG_IO_SIZE +#define MG_IO_SIZE 1460 +#endif + +#endif // MG_ARCH == MG_ARCH_RTTHREAD #if MG_ARCH == MG_ARCH_ARMCC || MG_ARCH == MG_ARCH_CMSIS_RTOS1 || \ @@ -859,7 +859,7 @@ char *mg_hex(const void *buf, size_t len, char *dst); void mg_unhex(const char *buf, size_t len, unsigned char *to); unsigned long mg_unhexn(const char *s, size_t len); int mg_check_ip_acl(struct mg_str acl, uint32_t remote_ip); -char *mg_remove_double_dots(char *s); +bool mg_path_is_sane(const char *path); diff --git a/src/http.c b/src/http.c index 8e7bbcca..6eef0c94 100644 --- a/src/http.c +++ b/src/http.c @@ -610,6 +610,7 @@ struct printdirentrydata { const char *dir; }; +#if MG_ENABLE_DIRLIST static void printdirentry(const char *name, void *userdata) { struct printdirentrydata *d = (struct printdirentrydata *) userdata; struct mg_fs *fs = d->opts->fs == NULL ? &mg_fs_posix : d->opts->fs; @@ -716,6 +717,7 @@ static void listdir(struct mg_connection *c, struct mg_http_message *hm, memcpy(c->send.buf + off - 12, tmp, n); // Set content length c->is_resp = 0; // Mark response end } +#endif // Resolve requested file into `path` and return its fs->st() result static int uri_to_path2(struct mg_connection *c, struct mg_http_message *hm, @@ -724,13 +726,20 @@ static int uri_to_path2(struct mg_connection *c, struct mg_http_message *hm, int flags, tmp; // Append URI to the root_dir, and sanitize it size_t n = mg_snprintf(path, path_size, "%.*s", (int) dir.len, dir.ptr); - if (n > path_size) n = path_size; + if (n > path_size) { + mg_http_reply(c, 400, "", "Exceeded path size"); + return -1; + } path[path_size - 1] = '\0'; - if (n + 2 < path_size) path[n++] = '/', path[n] = '\0'; + // Terminate root dir with / + if (n + 2 < path_size && path[n-1] != '/') path[n++] = '/', path[n] = '\0'; mg_url_decode(hm->uri.ptr + url.len, hm->uri.len - url.len, path + n, path_size - n, 0); path[path_size - 1] = '\0'; // Double-check - mg_remove_double_dots(path); + if (!mg_path_is_sane(path)) { + mg_http_reply(c, 400, "", "Invalid path"); + return -1; + } n = strlen(path); while (n > 1 && path[n - 1] == '/') path[--n] = 0; // Trim trailing slashes flags = mg_vcmp(&hm->uri, "/") == 0 ? MG_FS_DIR : fs->st(path, NULL, NULL); @@ -790,7 +799,11 @@ void mg_http_serve_dir(struct mg_connection *c, struct mg_http_message *hm, if (flags < 0) { // Do nothing: the response has already been sent by uri_to_path() } else if (flags & MG_FS_DIR) { +#if MG_ENABLE_DIRLIST listdir(c, hm, opts, path); +#else + mg_http_reply(c, 403, "", "Forbidden\n"); +#endif } else if (flags && sp != NULL && mg_globmatch(sp, strlen(sp), path, strlen(path))) { mg_http_serve_ssi(c, opts->root_dir, path); diff --git a/src/str.c b/src/str.c index f7f00e70..39002f51 100644 --- a/src/str.c +++ b/src/str.c @@ -187,23 +187,12 @@ void mg_unhex(const char *buf, size_t len, unsigned char *to) { } } -char *mg_remove_double_dots(char *s) { - char *saved = s, *p = s; - while (*s != '\0') { - *p++ = *s++; - if (s[-1] == '/' || s[-1] == '\\') { - while (s[0] != '\0') { - if (s[0] == '/' || s[0] == '\\') { - s++; - } else if (s[0] == '.' && s[1] == '.' && - (s[2] == '/' || s[2] == '\\')) { - s += 2; - } else { - break; - } - } +bool mg_path_is_sane(const char *path) { + const char *s = path; + for (; s[0] != '\0'; s++) { + if (s == path || s[0] == '/' || s[0] == '\\') { // Subdir? + if (s[1] == '.' && s[2] == '.') return false; // Starts with .. } } - *p = '\0'; - return saved; + return true; } diff --git a/src/str.h b/src/str.h index 00554781..97330ca6 100644 --- a/src/str.h +++ b/src/str.h @@ -35,4 +35,4 @@ char *mg_hex(const void *buf, size_t len, char *dst); void mg_unhex(const char *buf, size_t len, unsigned char *to); unsigned long mg_unhexn(const char *s, size_t len); int mg_check_ip_acl(struct mg_str acl, uint32_t remote_ip); -char *mg_remove_double_dots(char *s); +bool mg_path_is_sane(const char *path); diff --git a/test/unit_test.c b/test/unit_test.c index 1acb23e3..04567104 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -643,11 +643,13 @@ static void eh1(struct mg_connection *c, int ev, void *ev_data, void *fn_data) { } else if (mg_http_match_uri(hm, "/upload")) { char path[80], name[64]; mg_http_get_var(&hm->query, "name", name, sizeof(name)); + mg_snprintf(path, sizeof(path), "./%s", name); if (name[0] == '\0') { mg_http_reply(c, 400, "", "%s", "name required"); + } else if (!mg_path_is_sane(path)) { + mg_http_reply(c, 400, "", "%s", "invalid path"); } else { - mg_snprintf(path, sizeof(path), "./%s", name); - mg_http_upload(c, hm, &mg_fs_posix, mg_remove_double_dots(path), 99999); + mg_http_upload(c, hm, &mg_fs_posix, path, 99999); c->is_hexdumping = 1; } } else if (mg_http_match_uri(hm, "/test/")) { @@ -854,11 +856,11 @@ static void test_http_server(void) { ASSERT(fetch(&mgr, buf, url, "GET /київ.txt HTTP/1.0\n\n") == 200); ASSERT(cmpbody(buf, "є\n") == 0); - ASSERT(fetch(&mgr, buf, url, "GET /../fuzz.c HTTP/1.0\n\n") == 404); - ASSERT(fetch(&mgr, buf, url, "GET /.%%2e/fuzz.c HTTP/1.0\n\n") == 404); - ASSERT(fetch(&mgr, buf, url, "GET /.%%2e%%2ffuzz.c HTTP/1.0\n\n") == 404); - ASSERT(fetch(&mgr, buf, url, "GET /..%%2f%%20fuzz.c HTTP/1.0\n\n") == 404); - ASSERT(fetch(&mgr, buf, url, "GET /..%%2ffuzz.c%%20 HTTP/1.0\n\n") == 404); + ASSERT(fetch(&mgr, buf, url, "GET /../fuzz.c HTTP/1.0\n\n") == 400); + ASSERT(fetch(&mgr, buf, url, "GET /.%%2e/fuzz.c HTTP/1.0\n\n") == 400); + ASSERT(fetch(&mgr, buf, url, "GET /.%%2e%%2ffuzz.c HTTP/1.0\n\n") == 400); + ASSERT(fetch(&mgr, buf, url, "GET /..%%2f%%20fuzz.c HTTP/1.0\n\n") == 400); + ASSERT(fetch(&mgr, buf, url, "GET /..%%2ffuzz.c%%20 HTTP/1.0\n\n") == 400); ASSERT(fetch(&mgr, buf, url, "GET /dredir HTTP/1.0\n\n") == 301); ASSERT(cmpheader(buf, "Location", "/dredir/")); @@ -884,9 +886,8 @@ static void test_http_server(void) { ASSERT(cmpheader(buf, "Content-Encoding", "gzip") == true); ASSERT(gethm(buf).body.len == 23); - ASSERT(fetch(&mgr, buf, url, "GET /..ddot HTTP/1.0\n\n") == 301); - ASSERT(fetch(&mgr, buf, url, "GET /..ddot/ HTTP/1.0\n\n") == 200); - ASSERT(cmpbody(buf, "hi\n") == 0); + ASSERT(fetch(&mgr, buf, url, "GET /..ddot HTTP/1.0\n\n") == 400); + ASSERT(fetch(&mgr, buf, url, "GET /..ddot/ HTTP/1.0\n\n") == 400); ASSERT(fetch(&mgr, buf, url, "GET /a.txt HTTP/1.0\n" "Content-Length: -123\n\n") == 0); @@ -1048,7 +1049,7 @@ static void test_http_server(void) { remove("uploaded.txt"); ASSERT((p = mg_file_read(&mg_fs_posix, "uploaded.txt", NULL)) == NULL); ASSERT(fetch(&mgr, buf, url, - "POST /upload?name=../uploaded.txt HTTP/1.0\r\n" + "POST /upload?name=uploaded.txt HTTP/1.0\r\n" "Content-Length: 5\r\n" "\r\nhello") == 200); ASSERT((p = mg_file_read(&mg_fs_posix, "uploaded.txt", NULL)) != NULL);