From f25f08a49c32ca79e47f613b5bcd3d95328e8da3 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 18 Mar 2026 17:35:35 +0100 Subject: [PATCH 1/2] fix(file_sharing): prevent infinite loop on circular symlinks when duplicates are allowed --- src/file_sharing/directory_updater.cc | 48 +++++++++++++++++++-------- src/file_sharing/directory_updater.h | 3 +- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/file_sharing/directory_updater.cc b/src/file_sharing/directory_updater.cc index bb90c1b79f..dd3f4d61a9 100644 --- a/src/file_sharing/directory_updater.cc +++ b/src/file_sharing/directory_updater.cc @@ -24,6 +24,7 @@ #include "util/cxx17retrocompat.h" #include "util/folderiterator.h" +#include #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 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& existing_directories, uint32_t current_depth, + std::set& existing_directories, std::vector& 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 diff --git a/src/file_sharing/directory_updater.h b/src/file_sharing/directory_updater.h index b7d72b3a93..28aba0caff 100644 --- a/src/file_sharing/directory_updater.h +++ b/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 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& existing_directories, uint32_t current_depth,bool& files_not_ready); + void recursUpdateSharedDir(const std::string& cumulated_path, DirectoryStorage::EntryIndex indx, std::set& existing_directories, std::vector& current_branch_real_paths, uint32_t current_depth,bool& files_not_ready); bool sweepSharedDirectories(bool &some_files_not_ready); private: From d73b18b544d678123449df9b1f8a17eca0fa6be0 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Mon, 30 Mar 2026 09:35:38 +0200 Subject: [PATCH 2/2] fix(file_sharing): handle NULL from canonicalize_file_name and separate cycle detection from duplicate filtering --- src/file_sharing/directory_updater.cc | 25 +++++++++++++++++-------- src/file_sharing/directory_updater.h | 3 +-- src/util/rsdir.cc | 9 ++++++++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/file_sharing/directory_updater.cc b/src/file_sharing/directory_updater.cc index dd3f4d61a9..354cdff9f4 100644 --- a/src/file_sharing/directory_updater.cc +++ b/src/file_sharing/directory_updater.cc @@ -24,7 +24,6 @@ #include "util/cxx17retrocompat.h" #include "util/folderiterator.h" -#include #include "util/rstime.h" #include "rsserver/p3face.h" #include "directory_storage.h" @@ -221,9 +220,11 @@ bool LocalDirectoryUpdater::sweepSharedDirectories(bool& some_files_not_ready) RS_DBG4("recursing into \"", stored_dir_it.name()); std::string canonical = RsDirUtil::removeSymLinks(stored_dir_it.name()); - existing_dirs.insert(canonical); - std::vector current_branch_real_paths; - current_branch_real_paths.push_back(canonical); + if(!canonical.empty()) + existing_dirs.insert(canonical); + std::set current_branch_real_paths; + if(!canonical.empty()) + current_branch_real_paths.insert(canonical); recursUpdateSharedDir( stored_dir_it.name(), *stored_dir_it, existing_dirs, current_branch_real_paths, 1, some_files_not_ready ); @@ -244,7 +245,7 @@ bool LocalDirectoryUpdater::sweepSharedDirectories(bool& some_files_not_ready) void LocalDirectoryUpdater::recursUpdateSharedDir( const std::string& cumulated_path, DirectoryStorage::EntryIndex indx, - std::set& existing_directories, std::vector& current_branch_real_paths, uint32_t current_depth, + std::set& existing_directories, std::set& current_branch_real_paths, uint32_t current_depth, bool& some_files_not_ready ) { RS_DBG4("parsing directory \"", cumulated_path, "\" index: ", indx); @@ -309,7 +310,13 @@ void LocalDirectoryUpdater::recursUpdateSharedDir( std::string real_path = RsDirUtil::removeSymLinks( RsDirUtil::makePath(cumulated_path, dirIt.file_name()) ); - if (std::find(current_branch_real_paths.begin(), current_branch_real_paths.end(), real_path) != current_branch_real_paths.end()) + if(real_path.empty()) + { + RS_WARN( "Broken/circular symlink: \"", cumulated_path, + "/", dirIt.file_name(), "\". Ignoring." ); + dir_is_accepted = false; + } + else if(current_branch_real_paths.end() != current_branch_real_paths.find(real_path)) { RS_WARN( "Circular symlink detected: \"", cumulated_path, "\" points to ancestor \"", real_path, @@ -381,13 +388,15 @@ void LocalDirectoryUpdater::recursUpdateSharedDir( { 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); + if(!canonical.empty()) + current_branch_real_paths.insert(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(); + if(!canonical.empty()) + current_branch_real_paths.erase(canonical); } } diff --git a/src/file_sharing/directory_updater.h b/src/file_sharing/directory_updater.h index 28aba0caff..b49992b5eb 100644 --- a/src/file_sharing/directory_updater.h +++ b/src/file_sharing/directory_updater.h @@ -28,7 +28,6 @@ #include "file_sharing/hash_cache.h" #include "file_sharing/directory_storage.h" #include "util/rstime.h" -#include class LocalDirectoryUpdater: public HashStorageClient, public RsTickingThread { @@ -68,7 +67,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& existing_directories, std::vector& current_branch_real_paths, uint32_t current_depth,bool& files_not_ready); + void recursUpdateSharedDir(const std::string& cumulated_path, DirectoryStorage::EntryIndex indx, std::set& existing_directories, std::set& current_branch_real_paths, uint32_t current_depth,bool& files_not_ready); bool sweepSharedDirectories(bool &some_files_not_ready); private: diff --git a/src/util/rsdir.cc b/src/util/rsdir.cc index 0f7f5d9553..5cc1f8edf5 100644 --- a/src/util/rsdir.cc +++ b/src/util/rsdir.cc @@ -622,8 +622,15 @@ std::string RsDirUtil::removeSymLinks(const std::string& path) return path ; #else char *tmp = canonicalize_file_name(path.c_str()) ; - std::string result(tmp) ; + if(tmp == nullptr) + { + RS_WARN("removeSymLinks: cannot resolve \"", path, + "\". Broken or circular symlink?"); + return std::string(); + } + + std::string result(tmp) ; free(tmp); return result ; #endif