Skip to content

Commit

Permalink
HPCC-31314 Fix path traversal vulnerability in ESP
Browse files Browse the repository at this point in the history
1. Move the checkHttpPathStaysWithinBounds() so that it can be called
by other functions. 2. Call the checkHttpPathStaysWithinBounds()
before calling EspHttpBinding::onGet() if the method is 'files_'. 3.
Call the checkHttpPathStaysWithinBounds() in CEspHttpServer::onGetXslt.

Signed-off-by: wangkx <[email protected]>
  • Loading branch information
wangkx committed Feb 21, 2024
1 parent 3bfc741 commit 3814854
Showing 1 changed file with 44 additions and 27 deletions.
71 changes: 44 additions & 27 deletions esp/bindings/http/platform/httpservice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,33 @@ static bool authenticateOptionalFailed(IEspContext& ctx, IEspHttpBinding* bindin
return false;
}

static bool checkHttpPathStaysWithinBounds(const char *path)
{
if (isEmptyString(path))
return true;
//The path that follows /esp/files should be relative, not absolute - reject immediately if it is.
if (isAbsolutePath(path))
return false;
int depth = 0;
StringArray nodes;
nodes.appendList(path, "/");
ForEachItemIn(i, nodes)
{
const char *node = nodes.item(i);
if (!*node || streq(node, ".")) //empty or "." doesn't advance
continue;
if (!streq(node, ".."))
depth++;
else
{
depth--;
if (depth<0) //only really care that the relative http path doesn't position itself above its own root node
return false;
}
}
return true;
}

EspHttpBinding* CEspHttpServer::getBinding()
{
EspHttpBinding* thebinding=NULL;
Expand Down Expand Up @@ -401,7 +428,16 @@ int CEspHttpServer::processRequest()
m_response->redirect(*m_request.get(),url);
}
else
{
if (strieq(methodName.str(), "files_") && !checkHttpPathStaysWithinBounds(pathEx))
{
AERRLOG("Get File %s: attempted access outside of %sfiles/", pathEx.str(), getCFD());
m_response->setStatus(HTTP_STATUS_NOT_FOUND);
m_response->send();
return 0;
}
thebinding->onGet(m_request.get(), m_response.get());
}
}
else
unsupported();
Expand Down Expand Up @@ -716,33 +752,6 @@ static void httpGetDirectory(CHttpRequest* request, CHttpResponse* response, con
response->send();
}

static bool checkHttpPathStaysWithinBounds(const char *path)
{
if (!path || !*path)
return true;
//The path that follows /esp/files should be relative, not absolute - reject immediately if it is.
if (isAbsolutePath(path))
return false;
int depth = 0;
StringArray nodes;
nodes.appendList(path, "/");
ForEachItemIn(i, nodes)
{
const char *node = nodes.item(i);
if (!*node || streq(node, ".")) //empty or "." doesn't advance
continue;
if (!streq(node, ".."))
depth++;
else
{
depth--;
if (depth<0) //only really care that the relative http path doesn't position itself above its own root node
return false;
}
}
return true;
}

int CEspHttpServer::onGetFile(CHttpRequest* request, CHttpResponse* response, const char *urlpath)
{
if (!request || !response || !urlpath)
Expand Down Expand Up @@ -793,6 +802,14 @@ int CEspHttpServer::onGetXslt(CHttpRequest* request, CHttpResponse* response, co
if (!request || !response || !path)
return -1;

if (!checkHttpPathStaysWithinBounds(path))
{
AERRLOG("Get File %s: attempted access outside of %sxslt/", path, getCFD());
response->setStatus(HTTP_STATUS_NOT_FOUND);
response->send();
return 0;
}

StringBuffer mimetype, etag, lastModified;
MemoryBuffer content;
bool modified = true;
Expand Down

0 comments on commit 3814854

Please sign in to comment.