Merge remote-tracking branch 'origin/candidate-9.0.x' into candidate-…

…9.2.x Signed-off-by: Jake Smith <[email protected]> # Conflicts: # helm/hpcc/Chart.yaml # helm/hpcc/templates/_helpers.tpl # helm/hpcc/templates/dafilesrv.yaml # helm/hpcc/templates/dali.yaml # helm/hpcc/templates/dfuserver.yaml # helm/hpcc/templates/eclagent.yaml # helm/hpcc/templates/eclccserver.yaml # helm/hpcc/templates/eclscheduler.yaml # helm/hpcc/templates/esp.yaml # helm/hpcc/templates/localroxie.yaml # helm/hpcc/templates/roxie.yaml # helm/hpcc/templates/sasha.yaml # helm/hpcc/templates/thor.yaml # version.cmake
hpcc-systems · Feb 22, 2024 · d20e534 · d20e534
2 parents 7026577 + 8ed6d98
commit d20e534
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 35 deletions.
diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp
@@ -9894,6 +9894,7 @@ class CInitGroups
     StringArray clusternames;
     unsigned defaultTimeout;
     bool machinesLoaded;
+    bool writeLock;
 
     GroupType getGroupType(const char *type)
     {
@@ -9969,6 +9970,8 @@ class CInitGroups
 
     void addClusterGroup(const char *name, IPropertyTree *newClusterGroup, bool realCluster)
     {
+        if (!writeLock)
+            throw makeStringException(0, "CInitGroups::addClusterGroup called in read-only mode");
         VStringBuffer prop("Group[@name=\"%s\"]", name);
         IPropertyTree *root = groupsconnlock.conn->queryRoot();
         IPropertyTree *old = root->queryPropTree(prop.str());
@@ -10326,11 +10329,12 @@ class CInitGroups
         return root->queryPropTree(xpath.str());
     }
 public:
-    CInitGroups(unsigned _defaultTimeout)
-        : groupsconnlock("constructGroup",SDS_GROUPSTORE_ROOT,true,false,false,_defaultTimeout)
+    CInitGroups(unsigned _defaultTimeout, bool _writeLock)
+        : groupsconnlock("constructGroup",SDS_GROUPSTORE_ROOT,_writeLock,false,false,_defaultTimeout)
     {
         defaultTimeout = _defaultTimeout;
         machinesLoaded = false;
+        writeLock = _writeLock;
     }
 
     IPropertyTree *queryCluster(const IPropertyTree *env, const char *_clusterName, const char *type, const char *msg, StringBuffer &messages)
@@ -10667,13 +10671,13 @@ class CInitGroups
 
 void initClusterGroups(bool force, StringBuffer &response, IPropertyTree *oldEnvironment, unsigned timems)
 {
-    CInitGroups init(timems);
+    CInitGroups init(timems, true);
     init.constructGroups(force, response, oldEnvironment);
 }
 
 void initClusterAndStoragePlaneGroups(bool force, IPropertyTree *oldEnvironment, unsigned timems)
 {
-    CInitGroups init(timems);
+    CInitGroups init(timems, true);
 
     StringBuffer response;
     init.constructGroups(force, response, oldEnvironment);
@@ -10688,19 +10692,19 @@ void initClusterAndStoragePlaneGroups(bool force, IPropertyTree *oldEnvironment,
 
 bool resetClusterGroup(const char *clusterName, const char *type, bool spares, StringBuffer &response, unsigned timems)
 {
-    CInitGroups init(timems);
+    CInitGroups init(timems, true);
     return init.resetClusterGroup(clusterName, type, spares, response);
 }
 
 bool addClusterSpares(const char *clusterName, const char *type, const std::vector<std::string> &hosts, StringBuffer &response, unsigned timems)
 {
-    CInitGroups init(timems);
+    CInitGroups init(timems, true);
     return init.addSpares(clusterName, type, hosts, response);
 }
 
 bool removeClusterSpares(const char *clusterName, const char *type, const std::vector<std::string> &hosts, StringBuffer &response, unsigned timems)
 {
-    CInitGroups init(timems);
+    CInitGroups init(timems, true);
     return init.removeSpares(clusterName, type, hosts, response);
 }
 
@@ -10724,7 +10728,7 @@ static IGroup *getClusterNodeGroup(const char *clusterName, const char *type, bo
      * to DFS and elsewhere.
      */
     Owned<IGroup> nodeGroup = queryNamedGroupStore().lookup(nodeGroupName);
-    CInitGroups init(timems);
+    CInitGroups init(timems, false);
     Owned<IGroup> expandedClusterGroup = init.getGroupFromCluster(type, cluster, true);
     if (!expandedClusterGroup)
         throwStringExceptionV(0, "Failed to get group for '%s' cluster '%s'", type, clusterName);

diff --git 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;
@@ -408,7 +435,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();
@@ -724,33 +760,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)
@@ -801,6 +810,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;