Skip to content

Commit c65c8fd

Browse files
cpqcpq
committed
Protect against the directory traversal in mg_upload()
1 parent b484123 commit c65c8fd

File tree

4 files changed

+72
-58
lines changed

4 files changed

+72
-58
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ fuzz: fuzzer
5959
$(RUN) ./fuzzer
6060

6161
# make CC=/usr/local/opt/llvm\@8/bin/clang ASAN_OPTIONS=detect_leaks=1
62-
test: CFLAGS += -DMG_ENABLE_IPV6=$(IPV6) -fsanitize=address#,undefined
62+
test: CFLAGS += -DMG_ENABLE_IPV6=$(IPV6) -fsanitize=address,undefined
6363
test: mongoose.h Makefile $(SRCS)
6464
$(CC) $(SRCS) $(CFLAGS) -coverage $(LDFLAGS) -g -o unit_test
6565
ASAN_OPTIONS=$(ASAN_OPTIONS) $(RUN) ./unit_test

‎mongoose.c‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,34 +1142,6 @@ char *mg_http_etag(char *buf, size_t len, size_t size, time_t mtime) {
11421142
return buf;
11431143
}
11441144

1145-
#if MG_ENABLE_FILE
1146-
int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm,
1147-
const char *dir) {
1148-
char offset[40] = "", name[200] = "", path[256];
1149-
mg_http_get_var(&hm->query, "offset", offset, sizeof(offset));
1150-
mg_http_get_var(&hm->query, "name", name, sizeof(name));
1151-
if (name[0] == '\0') {
1152-
mg_http_reply(c, 400, "", "%s", "name required");
1153-
return -1;
1154-
} else {
1155-
FILE *fp;
1156-
size_t oft = strtoul(offset, NULL, 0);
1157-
snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name);
1158-
LOG(LL_DEBUG,
1159-
("%p %d bytes @ %d [%s]", c->fd, (int) hm->body.len, (int) oft, name));
1160-
if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) {
1161-
mg_http_reply(c, 400, "", "fopen(%s): %d", name, errno);
1162-
return -2;
1163-
} else {
1164-
fwrite(hm->body.ptr, 1, hm->body.len, fp);
1165-
fclose(fp);
1166-
mg_http_reply(c, 200, "", "");
1167-
return (int) hm->body.len;
1168-
}
1169-
}
1170-
}
1171-
#endif
1172-
11731145
static void static_cb(struct mg_connection *c, int ev, void *ev_data,
11741146
void *fn_data) {
11751147
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) {
16801652
c->recv.len -= ch.len;
16811653
}
16821654

1655+
#if MG_ENABLE_FILE
1656+
int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm,
1657+
const char *dir) {
1658+
char offset[40] = "", name[200] = "", path[256];
1659+
mg_http_get_var(&hm->query, "offset", offset, sizeof(offset));
1660+
mg_http_get_var(&hm->query, "name", name, sizeof(name));
1661+
if (name[0] == '\0') {
1662+
mg_http_reply(c, 400, "", "%s", "name required");
1663+
return -1;
1664+
} else {
1665+
FILE *fp;
1666+
long oft = strtol(offset, NULL, 0);
1667+
snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name);
1668+
remove_double_dots(path);
1669+
LOG(LL_DEBUG, ("%d bytes @ %ld [%s]", (int) hm->body.len, oft, path));
1670+
if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) {
1671+
mg_http_reply(c, 400, "", "fopen(%s): %d", path, errno);
1672+
return -2;
1673+
} else {
1674+
fwrite(hm->body.ptr, 1, hm->body.len, fp);
1675+
fclose(fp);
1676+
mg_http_reply(c, 200, "", "");
1677+
return (int) hm->body.len;
1678+
}
1679+
}
1680+
}
1681+
#endif
1682+
16831683
static void http_cb(struct mg_connection *c, int ev, void *evd, void *fnd) {
16841684
if (ev == MG_EV_READ || ev == MG_EV_CLOSE) {
16851685
struct mg_http_message hm;

‎src/http.c‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -378,34 +378,6 @@ char *mg_http_etag(char *buf, size_t len, size_t size, time_t mtime) {
378378
return buf;
379379
}
380380

381-
#if MG_ENABLE_FILE
382-
int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm,
383-
const char *dir) {
384-
char offset[40] = "", name[200] = "", path[256];
385-
mg_http_get_var(&hm->query, "offset", offset, sizeof(offset));
386-
mg_http_get_var(&hm->query, "name", name, sizeof(name));
387-
if (name[0] == '\0') {
388-
mg_http_reply(c, 400, "", "%s", "name required");
389-
return -1;
390-
} else {
391-
FILE *fp;
392-
size_t oft = strtoul(offset, NULL, 0);
393-
snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name);
394-
LOG(LL_DEBUG,
395-
("%p %d bytes @ %d [%s]", c->fd, (int) hm->body.len, (int) oft, name));
396-
if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) {
397-
mg_http_reply(c, 400, "", "fopen(%s): %d", name, errno);
398-
return -2;
399-
} else {
400-
fwrite(hm->body.ptr, 1, hm->body.len, fp);
401-
fclose(fp);
402-
mg_http_reply(c, 200, "", "");
403-
return (int) hm->body.len;
404-
}
405-
}
406-
}
407-
#endif
408-
409381
static void static_cb(struct mg_connection *c, int ev, void *ev_data,
410382
void *fn_data) {
411383
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) {
916888
c->recv.len -= ch.len;
917889
}
918890

891+
#if MG_ENABLE_FILE
892+
int mg_http_upload(struct mg_connection *c, struct mg_http_message *hm,
893+
const char *dir) {
894+
char offset[40] = "", name[200] = "", path[256];
895+
mg_http_get_var(&hm->query, "offset", offset, sizeof(offset));
896+
mg_http_get_var(&hm->query, "name", name, sizeof(name));
897+
if (name[0] == '\0') {
898+
mg_http_reply(c, 400, "", "%s", "name required");
899+
return -1;
900+
} else {
901+
FILE *fp;
902+
long oft = strtol(offset, NULL, 0);
903+
snprintf(path, sizeof(path), "%s%c%s", dir, MG_DIRSEP, name);
904+
remove_double_dots(path);
905+
LOG(LL_DEBUG, ("%d bytes @ %ld [%s]", (int) hm->body.len, oft, path));
906+
if ((fp = fopen(path, oft == 0 ? "wb" : "ab")) == NULL) {
907+
mg_http_reply(c, 400, "", "fopen(%s): %d", path, errno);
908+
return -2;
909+
} else {
910+
fwrite(hm->body.ptr, 1, hm->body.len, fp);
911+
fclose(fp);
912+
mg_http_reply(c, 200, "", "");
913+
return (int) hm->body.len;
914+
}
915+
}
916+
}
917+
#endif
918+
919919
static void http_cb(struct mg_connection *c, int ev, void *evd, void *fnd) {
920920
if (ev == MG_EV_READ || ev == MG_EV_CLOSE) {
921921
struct mg_http_message hm;

‎test/unit_test.c‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,6 @@ static void test_http_server(void) {
679679
char *p;
680680
remove("uploaded.txt");
681681
ASSERT((p = mg_file_read("uploaded.txt", NULL)) == NULL);
682-
683682
ASSERT(fetch(&mgr, buf, url,
684683
"POST /upload HTTP/1.0\n"
685684
"Content-Length: 1\n\nx") == 400);
@@ -698,6 +697,21 @@ static void test_http_server(void) {
698697
remove("uploaded.txt");
699698
}
700699

700+
{
701+
// Test upload directory traversal
702+
char *p;
703+
remove("uploaded.txt");
704+
ASSERT((p = mg_file_read("uploaded.txt", NULL)) == NULL);
705+
ASSERT(fetch(&mgr, buf, url,
706+
"POST /upload?name=../uploaded.txt HTTP/1.0\r\n"
707+
"Content-Length: 5\r\n"
708+
"\r\nhello") == 200);
709+
ASSERT((p = mg_file_read("uploaded.txt", NULL)) != NULL);
710+
ASSERT(strcmp(p, "hello") == 0);
711+
free(p);
712+
remove("uploaded.txt");
713+
}
714+
701715
// HEAD request
702716
ASSERT(fetch(&mgr, buf, url, "GET /a.txt HTTP/1.0\n\n") == 200);
703717
ASSERT(fetch(&mgr, buf, url, "HEAD /a.txt HTTP/1.0\n\n") == 200);

0 commit comments

Comments
 (0)