HPCC-29482 WsDfs should pass user to DFS::getFileTree

When performing a ~remote file lookup, the WsDfs service looks up the logical file in Dali (via IDistributedFileDirectory::getFileTree). It should pass the active esp context user, but passes null. This can cause access denials. Signed-off-by: Jake Smith <[email protected]>
hpcc-systems · Apr 5, 2024 · 82e97f1 · 82e97f1
1 parent 2f6621c
commit 82e97f1
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp
@@ -1256,7 +1256,7 @@ protected: friend class CDistributedFile;
         return ret;
     }
     virtual bool removePhysicalPartFiles(const char *logicalName, IFileDescriptor *fileDesc, IMultiException *mexcept, unsigned numParallelDeletes=0) override;
-    virtual void setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout);
+    virtual void setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout);
 };
 
 
@@ -11635,11 +11635,11 @@ void CDistributedFileDirectory::setFileAccessed(CDfsLogicalFileName &dlfn,IUserD
     checkDfsReplyException(mb);
 }
 
-void CDistributedFileDirectory::setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout)
+void CDistributedFileDirectory::setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout)
 {
     CDfsLogicalFileName dlfn;
     dlfn.set(logicalName);
-    setFileAccessed(dlfn, nullptr, dt, foreigndali, foreigndalitimeout);
+    setFileAccessed(dlfn, udesc, dt, foreigndali, foreigndalitimeout);
 }
 
 void CDistributedFileDirectory::setFileProtect(CDfsLogicalFileName &dlfn,IUserDescriptor *user, const char *owner, bool set, const INode *foreigndali,unsigned foreigndalitimeout)

diff --git a/dali/base/dadfs.hpp b/dali/base/dadfs.hpp
@@ -731,7 +731,7 @@ interface IDistributedFileDirectory: extends IInterface
     // useful to clearup after temporary unpublished file.
     virtual bool removePhysicalPartFiles(const char *logicalName, IFileDescriptor *fileDesc, IMultiException *mexcept, unsigned numParallelDeletes=0) = 0;
 
-    virtual void setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali=nullptr, unsigned foreigndalitimeout=FOREIGN_DALI_TIMEOUT) = 0;
+    virtual void setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali=nullptr, unsigned foreigndalitimeout=FOREIGN_DALI_TIMEOUT) = 0;
 };
 
 

diff --git a/esp/services/ws_dfsservice/ws_dfsservice.cpp b/esp/services/ws_dfsservice/ws_dfsservice.cpp
@@ -50,7 +50,7 @@ enum LfnMetaOpts : byte
     LfnMOptTls    = 0x02,
 };
 BITMASK_ENUM(LfnMetaOpts);
-static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, LfnMetaOpts opts, IPropertyTree *metaRoot, IPropertyTree *meta)
+static void populateLFNMeta(IUserDescriptor *userDesc, const char *logicalName, unsigned __int64 leaseId, LfnMetaOpts opts, IPropertyTree *metaRoot, IPropertyTree *meta)
 {
     CDfsLogicalFileName lfn;
     lfn.set(logicalName);
@@ -59,7 +59,8 @@ static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, L
 
     assertex(!lfn.isMulti()); // not supported, don't think needs to be/will be.
 
-    Owned<IPropertyTree> tree = queryDistributedFileDirectory().getFileTree(logicalName, nullptr);
+    StringBuffer userId;
+    Owned<IPropertyTree> tree = queryDistributedFileDirectory().getFileTree(logicalName, userDesc);
     if (!tree)
         return;
     if (hasMask(opts, LfnMOptRemap))
@@ -111,7 +112,7 @@ static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, L
             {
                 IPropertyTree &sub = *(orderedSubFiles[f]);
                 sub.getProp("@name", subname.clear());
-                populateLFNMeta(subname, leaseId, opts, metaRoot, fileMeta);
+                populateLFNMeta(userDesc, subname, leaseId, opts, metaRoot, fileMeta);
             }
         }
     }
@@ -182,7 +183,7 @@ bool CWsDfsEx::onDFSFileLookup(IEspContext &context, IEspDFSFileLookupRequest &r
             opts |= LfnMOptTls;
 
         Owned<IPropertyTree> responseTree = createPTree();
-        populateLFNMeta(logicalName, leaseId, opts, responseTree, responseTree);
+        populateLFNMeta(userDesc, logicalName, leaseId, opts, responseTree, responseTree);
 
         // serialize response
         MemoryBuffer respMb, compressedRespMb;
@@ -198,7 +199,7 @@ bool CWsDfsEx::onDFSFileLookup(IEspContext &context, IEspDFSFileLookupRequest &r
             //    Really this should be done at end (or at end as well), but this is same as existing DFS lookup.
             CDateTime dt;
             dt.setNow();
-            queryDistributedFileDirectory().setFileAccessed(logicalName, dt);
+            queryDistributedFileDirectory().setFileAccessed(userDesc, logicalName, dt);
 
             LOG(MCauditInfo,",FileAccess,EspProcess,READ,%s,%u,%s", logicalName, timeoutSecs, userID.str());
         }