fix(file_sharing): prevent infinite loop on circular symlinks when duplicates are allowed

Author: jolavillette

src/file_sharing/directory_updater.cc

@@ -24,6 +24,7 @@
 
 #include "util/cxx17retrocompat.h"
 #include "util/folderiterator.h"
+#include <algorithm>
 #include "util/rstime.h"
 #include "rsserver/p3face.h"
 #include "directory_storage.h"
@@ -219,10 +220,13 @@ bool LocalDirectoryUpdater::sweepSharedDirectories(bool& some_files_not_ready)
 	{
 		RS_DBG4("recursing into \"", stored_dir_it.name());
 
-		existing_dirs.insert(RsDirUtil::removeSymLinks(stored_dir_it.name()));
+		std::string canonical = RsDirUtil::removeSymLinks(stored_dir_it.name());
+		existing_dirs.insert(canonical);
+		std::vector<std::string> current_branch_real_paths;
+		current_branch_real_paths.push_back(canonical);
 		recursUpdateSharedDir(
 		            stored_dir_it.name(), *stored_dir_it,
-		            existing_dirs, 1, some_files_not_ready );
+		            existing_dirs, current_branch_real_paths, 1, some_files_not_ready );
 		/* here we need to use the list that was stored, instead of the shared
 		 * dir list, because the two are not necessarily in the same order. */
 	}
@@ -240,7 +244,7 @@ bool LocalDirectoryUpdater::sweepSharedDirectories(bool& some_files_not_ready)
 
 void LocalDirectoryUpdater::recursUpdateSharedDir(
         const std::string& cumulated_path, DirectoryStorage::EntryIndex indx,
-        std::set<std::string>& existing_directories, uint32_t current_depth,
+        std::set<std::string>& existing_directories, std::vector<std::string>& current_branch_real_paths, uint32_t current_depth,
         bool& some_files_not_ready )
 {
 	RS_DBG4("parsing directory \"", cumulated_path, "\" index: ", indx);
@@ -300,21 +304,31 @@ void LocalDirectoryUpdater::recursUpdateSharedDir(
 					        || (mMaxShareDepth == 0 && current_depth >= 64) )
 						dir_is_accepted = false;
 
-					if(dir_is_accepted && mFollowSymLinks && mIgnoreDuplicates)
+					if(dir_is_accepted && mFollowSymLinks)
 					{
 						std::string real_path = RsDirUtil::removeSymLinks(
 						            RsDirUtil::makePath(cumulated_path, dirIt.file_name()) );
 
-						if( existing_directories.end() !=
-						        existing_directories.find(real_path) )
+						if (std::find(current_branch_real_paths.begin(), current_branch_real_paths.end(), real_path) != current_branch_real_paths.end())
 						{
-							RS_WARN( "Directory: \"", cumulated_path,
-							         "\" has real path: \"", real_path,
-							         "\" which already belongs to another "
-							         "shared directory. Ignoring" );
+							RS_WARN( "Circular symlink detected: \"", cumulated_path,
+							         "\" points to ancestor \"", real_path,
+							         "\". Ignoring." );
 							dir_is_accepted = false;
 						}
-						else existing_directories.insert(real_path);
+						else if(mIgnoreDuplicates)
+						{
+							if( existing_directories.end() !=
+									existing_directories.find(real_path) )
+							{
+								RS_WARN( "Directory: \"", cumulated_path,
+										 "\" has real path: \"", real_path,
+										 "\" which already belongs to another "
+										 "shared directory. Ignoring" );
+								dir_is_accepted = false;
+							}
+							else existing_directories.insert(real_path);
+						}
 					}
 
 					if(dir_is_accepted) subdirs.insert(dirIt.file_name());
@@ -364,9 +378,17 @@ void LocalDirectoryUpdater::recursUpdateSharedDir(
 	// go through the list of sub-dirs and recursively update
 	for( DirectoryStorage::DirIterator stored_dir_it(mSharedDirectories, indx);
 	     stored_dir_it; ++stored_dir_it )
-		recursUpdateSharedDir( RsDirUtil::makePath(cumulated_path, stored_dir_it.name()),
-		                       *stored_dir_it, existing_directories,
+	{
+		std::string next_path = RsDirUtil::makePath(cumulated_path, stored_dir_it.name());
+		std::string canonical = RsDirUtil::removeSymLinks(next_path);
+		current_branch_real_paths.push_back(canonical);
+
+		recursUpdateSharedDir( next_path,
+		                       *stored_dir_it, existing_directories, current_branch_real_paths,
 		                       current_depth+1, some_files_not_ready );
+
+		current_branch_real_paths.pop_back();
+	}
 }
 
 bool LocalDirectoryUpdater::filterFile(const std::string& fname) const

src/file_sharing/directory_updater.h

@@ -28,6 +28,7 @@
 #include "file_sharing/hash_cache.h"
 #include "file_sharing/directory_storage.h"
 #include "util/rstime.h"
+#include <vector>
 
 class LocalDirectoryUpdater: public HashStorageClient, public RsTickingThread
 {
@@ -67,7 +68,7 @@ class LocalDirectoryUpdater: public HashStorageClient, public RsTickingThread
     virtual void hash_callback(uint32_t client_param, const std::string& name, const RsFileHash& hash, uint64_t size);
     virtual bool hash_confirm(uint32_t client_param) ;
 
-    void recursUpdateSharedDir(const std::string& cumulated_path, DirectoryStorage::EntryIndex indx, std::set<std::string>& existing_directories, uint32_t current_depth,bool& files_not_ready);
+    void recursUpdateSharedDir(const std::string& cumulated_path, DirectoryStorage::EntryIndex indx, std::set<std::string>& existing_directories, std::vector<std::string>& current_branch_real_paths, uint32_t current_depth,bool& files_not_ready);
     bool sweepSharedDirectories(bool &some_files_not_ready);
 
 private: