test/drogon
1
0
Fork 0
mirror of https://gitee.com/an-tao/drogon.git synced 2024-12-03 12:18:11 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Fix single layer directory traversal in StaticFileRouter (#901)

Browse Source 
The StaticFileRouter can access file in the immediate parent directory if the
client sends a specially crafted, non RFC conforming HTTP 1.x request. By
sending a HTTP request without a "/" predicating the path. The StaticFileRouter
fails to detect directory traversal since it checks for "/../" in the path.

This PR fixes the issue by detecting if there's potential for directory
traversal. If true, we follow the path and detect if it reaches out of the
document root at any point. Also added 2 new tests for edge cases in static
file serving. (Not related to the bug).

Co-authored-by: an-tao <antao2002@gmail.com>
Bug discovered by: oToToT <https://github.com/oToToT>
This commit is contained in:
Martin Chang 2021-06-24 13:04:19 +08:00 committed by GitHub
parent 065675486d
commit b8d820fc2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
lib/src/StaticFileRouter.cc
View File

@ -59,11 +59,28 @@ void StaticFileRouter::route(
std::function<void(const HttpResponsePtr &)> &&callback)
{
const std::string &path = req->path();
if (path.find("/../") != std::string::npos)
if (path.find("..") != std::string::npos)
{
// Downloading files from the parent folder is forbidden.
callback(app().getCustomErrorHandler()(k403Forbidden));
return;
auto directories = utils::splitString(path, "/");
int traversalDepth = 0;
for (const auto &dir : directories)
{
if (dir == "..")
{
traversalDepth--;
}
else if (dir != ".")
{
traversalDepth++;
}
if (traversalDepth < 0)
{
// Downloading files from the parent folder is forbidden.
callback(app().getCustomErrorHandler()(k403Forbidden));
return;
}
}
}
auto lPath = path;

26
lib/tests/integration_test/client/main.cc
View File

@ -681,6 +681,32 @@ void doTest(const HttpClientPtr &client, std::shared_ptr<test::Case> TEST_CTX)
CHECK((*json)["P2"] == "test");
});
// Using .. to access a upper directory should be permitted as long as
// it never leaves the document root
req = HttpRequest::newHttpRequest();
req->setMethod(drogon::Get);
req->setPath("/a-directory/../index.html");
client->sendRequest(req,
[req, TEST_CTX](ReqResult result,
const HttpResponsePtr &resp) {
REQUIRE(result == ReqResult::Ok);
CHECK(resp->getBody().length() == indexLen);
});
// . (current directory) shall also be allowed
req = HttpRequest::newHttpRequest();
req->setMethod(drogon::Get);
req->setPath("/a-directory/./page.html");
client->sendRequest(req,
[req, TEST_CTX, body](ReqResult result,
const HttpResponsePtr &resp) {
REQUIRE(result == ReqResult::Ok);
CHECK(resp->getBody().length() == indexImplicitLen);
CHECK(std::equal(body->begin(),
body->end(),
resp->getBody().begin()));
});
// Test exception handling
req = HttpRequest::newHttpRequest();
req->setMethod(drogon::Get);