diff --git a/examples/file-upload-multiple-posts/main.c b/examples/file-upload-multiple-posts/main.c index 8895da52f31..0e34474a5a7 100644 --- a/examples/file-upload-multiple-posts/main.c +++ b/examples/file-upload-multiple-posts/main.c @@ -10,16 +10,7 @@ static void cb(struct mg_connection *c, int ev, void *ev_data) { if (ev == MG_EV_HTTP_MSG) { struct mg_http_message *hm = (struct mg_http_message *) ev_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_http_upload(c, hm, &mg_fs_posix, path, 99999); - } + mg_http_upload(c, hm, &mg_fs_posix, "/tmp", 99999); } else { struct mg_http_serve_opts opts = {.root_dir = "web_root"}; mg_http_serve_dir(c, ev_data, &opts); diff --git a/examples/file-upload-multiple-posts/web_root/app.js b/examples/file-upload-multiple-posts/web_root/app.js index 98d871b1c78..6b247614b43 100644 --- a/examples/file-upload-multiple-posts/web_root/app.js +++ b/examples/file-upload-multiple-posts/web_root/app.js @@ -17,7 +17,7 @@ var sendFileData = function(name, data, chunkSize) { var sendChunk = function(offset) { var chunk = data.subarray(offset, offset + chunkSize) || ''; var opts = {method: 'POST', body: chunk}; - var url = '/upload?offset=' + offset + '&name=' + encodeURIComponent(name); + var url = '/upload?offset=' + offset + '&file=' + encodeURIComponent(name); var ok; setStatus( 'Uploading ' + name + ', bytes ' + offset + '..' + diff --git a/mongoose.c b/mongoose.c index 15ade247d1f..22615eec84f 100644 --- a/mongoose.c +++ b/mongoose.c @@ -2341,11 +2341,13 @@ struct mg_str mg_http_var(struct mg_str buf, struct mg_str name) { int mg_http_get_var(const struct mg_str *buf, const char *name, char *dst, size_t dst_len) { int len; + if (dst != NULL && dst_len > 0) { + dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it + } if (dst == NULL || dst_len == 0) { len = -2; // Bad destination } else if (buf->ptr == NULL || name == NULL || buf->len == 0) { len = -1; // Bad source - dst[0] = '\0'; } else { struct mg_str v = mg_http_var(*buf, mg_str(name)); if (v.ptr == NULL) { @@ -3129,32 +3131,40 @@ bool mg_http_match_uri(const struct mg_http_message *hm, const char *glob) { } long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - struct mg_fs *fs, const char *path, size_t max_size) { - char buf[20] = "0"; + struct mg_fs *fs, const char *dir, size_t max_size) { + char buf[20] = "0", file[40], path[MG_PATH_MAX]; long res = 0, offset; mg_http_get_var(&hm->query, "offset", buf, sizeof(buf)); + mg_http_get_var(&hm->query, "file", file, sizeof(file)); offset = strtol(buf, NULL, 0); + mg_snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, file); if (hm->body.len == 0) { mg_http_reply(c, 200, "", "%ld", res); // Nothing to write + } else if (file[0] == '\0') { + mg_http_reply(c, 400, "", "file required"); + res = -1; + } else if (mg_path_is_sane(file) == false) { + mg_http_reply(c, 400, "", "%s: invalid file", file); + res = -2; + } else if (offset < 0) { + mg_http_reply(c, 400, "", "offset required"); + res = -3; + } else if ((size_t) offset + hm->body.len > max_size) { + mg_http_reply(c, 400, "", "%s: over max size of %lu", path, + (unsigned long) max_size); + res = -4; } else { struct mg_fd *fd; size_t current_size = 0; - MG_DEBUG(("%s -> %d bytes @ %ld", path, (int) hm->body.len, offset)); + MG_DEBUG(("%s -> %lu bytes @ %ld", path, hm->body.len, offset)); if (offset == 0) fs->rm(path); // If offset if 0, truncate file fs->st(path, ¤t_size, NULL); - if (offset < 0) { - mg_http_reply(c, 400, "", "offset required"); - res = -1; - } else if (offset > 0 && current_size != (size_t) offset) { + if (offset > 0 && current_size != (size_t) offset) { mg_http_reply(c, 400, "", "%s: offset mismatch", path); - res = -2; - } else if ((size_t) offset + hm->body.len > max_size) { - mg_http_reply(c, 400, "", "%s: over max size of %lu", path, - (unsigned long) max_size); - res = -3; + res = -5; } else if ((fd = mg_fs_open(fs, path, MG_FS_WRITE)) == NULL) { mg_http_reply(c, 400, "", "open(%s): %d", path, errno); - res = -4; + res = -6; } else { res = offset + (long) fs->wr(fd->fd, hm->body.ptr, hm->body.len); mg_fs_close(fd); @@ -8052,8 +8062,9 @@ void mg_unhex(const char *buf, size_t len, unsigned char *to) { bool mg_path_is_sane(const char *path) { const char *s = path; + if (path[0] == '.' && path[1] == '.') return false; // Starts with .. for (; s[0] != '\0'; s++) { - if (s == path || s[0] == '/' || s[0] == '\\') { // Subdir? + if (s[0] == '/' || s[0] == '\\') { // Subdir? if (s[1] == '.' && s[2] == '.') return false; // Starts with .. } } diff --git a/mongoose.h b/mongoose.h index 7e708e371fd..9ac3edf7738 100644 --- a/mongoose.h +++ b/mongoose.h @@ -2291,7 +2291,7 @@ size_t mg_url_encode(const char *s, size_t n, char *buf, size_t len); void mg_http_creds(struct mg_http_message *, char *, size_t, char *, size_t); bool mg_http_match_uri(const struct mg_http_message *, const char *glob); long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - struct mg_fs *fs, const char *path, size_t max_size); + struct mg_fs *fs, const char *dir, size_t max_size); void mg_http_bauth(struct mg_connection *, const char *user, const char *pass); struct mg_str mg_http_get_header_var(struct mg_str s, struct mg_str v); size_t mg_http_next_multipart(struct mg_str, size_t, struct mg_http_part *); diff --git a/src/http.c b/src/http.c index b7c85ff0b13..14a7cb4bfa6 100644 --- a/src/http.c +++ b/src/http.c @@ -126,11 +126,13 @@ struct mg_str mg_http_var(struct mg_str buf, struct mg_str name) { int mg_http_get_var(const struct mg_str *buf, const char *name, char *dst, size_t dst_len) { int len; + if (dst != NULL && dst_len > 0) { + dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it + } if (dst == NULL || dst_len == 0) { len = -2; // Bad destination } else if (buf->ptr == NULL || name == NULL || buf->len == 0) { len = -1; // Bad source - dst[0] = '\0'; } else { struct mg_str v = mg_http_var(*buf, mg_str(name)); if (v.ptr == NULL) { @@ -914,32 +916,40 @@ bool mg_http_match_uri(const struct mg_http_message *hm, const char *glob) { } long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - struct mg_fs *fs, const char *path, size_t max_size) { - char buf[20] = "0"; + struct mg_fs *fs, const char *dir, size_t max_size) { + char buf[20] = "0", file[40], path[MG_PATH_MAX]; long res = 0, offset; mg_http_get_var(&hm->query, "offset", buf, sizeof(buf)); + mg_http_get_var(&hm->query, "file", file, sizeof(file)); offset = strtol(buf, NULL, 0); + mg_snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, file); if (hm->body.len == 0) { mg_http_reply(c, 200, "", "%ld", res); // Nothing to write + } else if (file[0] == '\0') { + mg_http_reply(c, 400, "", "file required"); + res = -1; + } else if (mg_path_is_sane(file) == false) { + mg_http_reply(c, 400, "", "%s: invalid file", file); + res = -2; + } else if (offset < 0) { + mg_http_reply(c, 400, "", "offset required"); + res = -3; + } else if ((size_t) offset + hm->body.len > max_size) { + mg_http_reply(c, 400, "", "%s: over max size of %lu", path, + (unsigned long) max_size); + res = -4; } else { struct mg_fd *fd; size_t current_size = 0; - MG_DEBUG(("%s -> %d bytes @ %ld", path, (int) hm->body.len, offset)); + MG_DEBUG(("%s -> %lu bytes @ %ld", path, hm->body.len, offset)); if (offset == 0) fs->rm(path); // If offset if 0, truncate file fs->st(path, ¤t_size, NULL); - if (offset < 0) { - mg_http_reply(c, 400, "", "offset required"); - res = -1; - } else if (offset > 0 && current_size != (size_t) offset) { + if (offset > 0 && current_size != (size_t) offset) { mg_http_reply(c, 400, "", "%s: offset mismatch", path); - res = -2; - } else if ((size_t) offset + hm->body.len > max_size) { - mg_http_reply(c, 400, "", "%s: over max size of %lu", path, - (unsigned long) max_size); - res = -3; + res = -5; } else if ((fd = mg_fs_open(fs, path, MG_FS_WRITE)) == NULL) { mg_http_reply(c, 400, "", "open(%s): %d", path, errno); - res = -4; + res = -6; } else { res = offset + (long) fs->wr(fd->fd, hm->body.ptr, hm->body.len); mg_fs_close(fd); diff --git a/src/http.h b/src/http.h index 0e810bb82bd..17a28b8be11 100644 --- a/src/http.h +++ b/src/http.h @@ -59,7 +59,7 @@ size_t mg_url_encode(const char *s, size_t n, char *buf, size_t len); void mg_http_creds(struct mg_http_message *, char *, size_t, char *, size_t); bool mg_http_match_uri(const struct mg_http_message *, const char *glob); long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, - struct mg_fs *fs, const char *path, size_t max_size); + struct mg_fs *fs, const char *dir, size_t max_size); void mg_http_bauth(struct mg_connection *, const char *user, const char *pass); struct mg_str mg_http_get_header_var(struct mg_str s, struct mg_str v); size_t mg_http_next_multipart(struct mg_str, size_t, struct mg_http_part *); diff --git a/src/str.c b/src/str.c index 39002f51c43..50ed034726d 100644 --- a/src/str.c +++ b/src/str.c @@ -189,8 +189,9 @@ void mg_unhex(const char *buf, size_t len, unsigned char *to) { bool mg_path_is_sane(const char *path) { const char *s = path; + if (path[0] == '.' && path[1] == '.') return false; // Starts with .. for (; s[0] != '\0'; s++) { - if (s == path || s[0] == '/' || s[0] == '\\') { // Subdir? + if (s[0] == '/' || s[0] == '\\') { // Subdir? if (s[1] == '.' && s[2] == '.') return false; // Starts with .. } } diff --git a/test/unit_test.c b/test/unit_test.c index 6edeeb93e82..2da5e44c38b 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -713,17 +713,8 @@ static void eh1(struct mg_connection *c, int ev, void *ev_data) { mg_http_creds(hm, user, sizeof(user), pass, sizeof(pass)); mg_http_reply(c, 200, "", "[%s]:[%s]", user, pass); } 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_http_upload(c, hm, &mg_fs_posix, path, 99999); - c->is_hexdumping = 1; - } + mg_http_upload(c, hm, &mg_fs_posix, ".", 99999); + c->is_hexdumping = 1; } else if (mg_http_match_uri(hm, "/test/")) { struct mg_http_serve_opts sopts; memset(&sopts, 0, sizeof(sopts)); @@ -1108,11 +1099,11 @@ static void test_http_server(void) { "Content-Length: 1\n\nx") == 400); ASSERT(fetch(&mgr, buf, url, - "POST /upload?name=uploaded.txt HTTP/1.0\r\n" + "POST /upload?file=uploaded.txt HTTP/1.0\r\n" "Content-Length: 5\r\n" "\r\nhello") == 200); ASSERT(fetch(&mgr, buf, url, - "POST /upload?name=uploaded.txt&offset=5 HTTP/1.0\r\n" + "POST /upload?file=uploaded.txt&offset=5 HTTP/1.0\r\n" "Content-Length: 6\r\n" "\r\n\nworld") == 200); ASSERT((p = mg_file_read(&mg_fs_posix, "uploaded.txt", NULL)) != NULL); @@ -1127,13 +1118,10 @@ 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?file=../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); - ASSERT(strcmp(p, "hello") == 0); - free(p); - remove("uploaded.txt"); + "\r\nhello") == 400); + ASSERT((p = mg_file_read(&mg_fs_posix, "uploaded.txt", NULL)) == NULL); } // HEAD request @@ -2120,6 +2108,21 @@ static void test_str(void) { 26); ASSERT(strcmp(buf, "[164:2100:0:0:0:0:0:0]:3 7") == 0); } + + ASSERT(mg_path_is_sane(".") == true); + ASSERT(mg_path_is_sane("") == true); + ASSERT(mg_path_is_sane("a.b") == true); + ASSERT(mg_path_is_sane("a..b") == true); + ASSERT(mg_path_is_sane(".a") == true); + ASSERT(mg_path_is_sane(".a.") == true); + ASSERT(mg_path_is_sane("./") == true); + ASSERT(mg_path_is_sane("a..") == true); + ASSERT(mg_path_is_sane("././a..") == true); + ASSERT(mg_path_is_sane("..") == false); + ASSERT(mg_path_is_sane("../") == false); + ASSERT(mg_path_is_sane("./..") == false); + ASSERT(mg_path_is_sane("a/../") == false); + ASSERT(mg_path_is_sane("a/../b") == false); } static void fn1(struct mg_connection *c, int ev, void *ev_data) {