From 3814854089c54193783c6b66d231bb6454466d4a Mon Sep 17 00:00:00 2001
From: wangkx <kevin.wang@lexisnexis.com>
Date: Wed, 21 Feb 2024 10:22:38 -0500
Subject: [PATCH] HPCC-31314 Fix path traversal vulnerability in ESP

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 <kevin.wang@lexisnexis.com>
---
 esp/bindings/http/platform/httpservice.cpp | 71 ++++++++++++++--------
 1 file changed, 44 insertions(+), 27 deletions(-)

diff --git a/esp/bindings/http/platform/httpservice.cpp b/esp/bindings/http/platform/httpservice.cpp
index 3b95f31f33a..361252493c0 100644
--- a/esp/bindings/http/platform/httpservice.cpp
+++ b/esp/bindings/http/platform/httpservice.cpp
@@ -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;
@@ -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();
@@ -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)
@@ -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;