From c65c8fdaaa257e0487ab0aaae9e8f6b439335945 Mon Sep 17 00:00:00 2001 From: Sergey Lyubka Date: Mon, 17 Jan 2022 12:08:23 +0000 Subject: [PATCH] Protect against the directory traversal in mg_upload() --- Makefile | 2 +- mongoose.c | 56 ++++++++++++++++++++++++------------------------ src/http.c | 56 ++++++++++++++++++++++++------------------------ test/unit_test.c | 16 +++++++++++++- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/Makefile b/Makefile index 2f2debcec6..93bea01417 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ fuzz: fuzzer $(RUN) ./fuzzer # make CC=/usr/local/opt/llvm\@8/bin/clang ASAN_OPTIONS=detect_leaks=1 -test: CFLAGS += -DMG_ENABLE_IPV6=$(IPV6) -fsanitize=address#,undefined +test: CFLAGS += -DMG_ENABLE_IPV6=$(IPV6) -fsanitize=address,undefined test: mongoose.h Makefile $(SRCS) $(CC) $(SRCS) $(CFLAGS) -coverage $(LDFLAGS) -g -o unit_test ASAN_OPTIONS=$(ASAN_OPTIONS) $(RUN) ./unit_test diff --git a/mongoose.c b/mongoose.c index 7e612608fd..2420dabb3a 100644 --- a/mongoose.c +++ b/mongoose.c @@ -1142,34 +1142,6 @@ char *mg_http_etag(char *buf, size_t len, size_t size, time_t mtime) { return buf; } -#if MG_ENABLE_FILE -int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - const char *dir) { - char offset[40] = "", name[200] = "", path[256]; - mg_http_get_var(&hm->query, "offset", offset, sizeof(offset)); - mg_http_get_var(&hm->query, "name", name, sizeof(name)); - if (name[0] == '\0') { - mg_http_reply(c, 400, "", "%s", "name required"); - return -1; - } else { - FILE *fp; - size_t oft = strtoul(offset, NULL, 0); - snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name); - LOG(LL_DEBUG, - ("%p %d bytes @ %d [%s]", c->fd, (int) hm->body.len, (int) oft, name)); - if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) { - mg_http_reply(c, 400, "", "fopen(%s): %d", name, errno); - return -2; - } else { - fwrite(hm->body.ptr, 1, hm->body.len, fp); - fclose(fp); - mg_http_reply(c, 200, "", ""); - return (int) hm->body.len; - } - } -} -#endif - static void static_cb(struct mg_connection *c, int ev, void *ev_data, void *fn_data) { if (ev == MG_EV_WRITE || ev == MG_EV_POLL) { @@ -1680,6 +1652,34 @@ void mg_http_delete_chunk(struct mg_connection *c, struct mg_http_message *hm) { c->recv.len -= ch.len; } +#if MG_ENABLE_FILE +int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, + const char *dir) { + char offset[40] = "", name[200] = "", path[256]; + mg_http_get_var(&hm->query, "offset", offset, sizeof(offset)); + mg_http_get_var(&hm->query, "name", name, sizeof(name)); + if (name[0] == '\0') { + mg_http_reply(c, 400, "", "%s", "name required"); + return -1; + } else { + FILE *fp; + long oft = strtol(offset, NULL, 0); + snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name); + remove_double_dots(path); + LOG(LL_DEBUG, ("%d bytes @ %ld [%s]", (int) hm->body.len, oft, path)); + if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) { + mg_http_reply(c, 400, "", "fopen(%s): %d", path, errno); + return -2; + } else { + fwrite(hm->body.ptr, 1, hm->body.len, fp); + fclose(fp); + mg_http_reply(c, 200, "", ""); + return (int) hm->body.len; + } + } +} +#endif + static void http_cb(struct mg_connection *c, int ev, void *evd, void *fnd) { if (ev == MG_EV_READ || ev == MG_EV_CLOSE) { struct mg_http_message hm; diff --git a/src/http.c b/src/http.c index 33fd94e29c..90c8213574 100644 --- a/src/http.c +++ b/src/http.c @@ -378,34 +378,6 @@ char *mg_http_etag(char *buf, size_t len, size_t size, time_t mtime) { return buf; } -#if MG_ENABLE_FILE -int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - const char *dir) { - char offset[40] = "", name[200] = "", path[256]; - mg_http_get_var(&hm->query, "offset", offset, sizeof(offset)); - mg_http_get_var(&hm->query, "name", name, sizeof(name)); - if (name[0] == '\0') { - mg_http_reply(c, 400, "", "%s", "name required"); - return -1; - } else { - FILE *fp; - size_t oft = strtoul(offset, NULL, 0); - snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name); - LOG(LL_DEBUG, - ("%p %d bytes @ %d [%s]", c->fd, (int) hm->body.len, (int) oft, name)); - if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) { - mg_http_reply(c, 400, "", "fopen(%s): %d", name, errno); - return -2; - } else { - fwrite(hm->body.ptr, 1, hm->body.len, fp); - fclose(fp); - mg_http_reply(c, 200, "", ""); - return (int) hm->body.len; - } - } -} -#endif - static void static_cb(struct mg_connection *c, int ev, void *ev_data, void *fn_data) { if (ev == MG_EV_WRITE || ev == MG_EV_POLL) { @@ -916,6 +888,34 @@ void mg_http_delete_chunk(struct mg_connection *c, struct mg_http_message *hm) { c->recv.len -= ch.len; } +#if MG_ENABLE_FILE +int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, + const char *dir) { + char offset[40] = "", name[200] = "", path[256]; + mg_http_get_var(&hm->query, "offset", offset, sizeof(offset)); + mg_http_get_var(&hm->query, "name", name, sizeof(name)); + if (name[0] == '\0') { + mg_http_reply(c, 400, "", "%s", "name required"); + return -1; + } else { + FILE *fp; + long oft = strtol(offset, NULL, 0); + snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name); + remove_double_dots(path); + LOG(LL_DEBUG, ("%d bytes @ %ld [%s]", (int) hm->body.len, oft, path)); + if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) { + mg_http_reply(c, 400, "", "fopen(%s): %d", path, errno); + return -2; + } else { + fwrite(hm->body.ptr, 1, hm->body.len, fp); + fclose(fp); + mg_http_reply(c, 200, "", ""); + return (int) hm->body.len; + } + } +} +#endif + static void http_cb(struct mg_connection *c, int ev, void *evd, void *fnd) { if (ev == MG_EV_READ || ev == MG_EV_CLOSE) { struct mg_http_message hm; diff --git a/test/unit_test.c b/test/unit_test.c index b41f3a74be..e366bbc3ea 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -679,7 +679,6 @@ static void test_http_server(void) { char *p; remove("uploaded.txt"); ASSERT((p = mg_file_read("uploaded.txt", NULL)) == NULL); - ASSERT(fetch(&mgr, buf, url, "POST /upload HTTP/1.0\n" "Content-Length: 1\n\nx") == 400); @@ -698,6 +697,21 @@ static void test_http_server(void) { remove("uploaded.txt"); } + { + // Test upload directory traversal + char *p; + remove("uploaded.txt"); + ASSERT((p = mg_file_read("uploaded.txt", NULL)) == NULL); + ASSERT(fetch(&mgr, buf, url, + "POST /upload?name=../uploaded.txt HTTP/1.0\r\n" + "Content-Length: 5\r\n" + "\r\nhello") == 200); + ASSERT((p = mg_file_read("uploaded.txt", NULL)) != NULL); + ASSERT(strcmp(p, "hello") == 0); + free(p); + remove("uploaded.txt"); + } + // HEAD request ASSERT(fetch(&mgr, buf, url, "GET /a.txt HTTP/1.0\n\n") == 200); ASSERT(fetch(&mgr, buf, url, "HEAD /a.txt HTTP/1.0\n\n") == 200);