improve file_storage::sanitize_symlinks

ChangeLog

@@ -1,3 +1,4 @@
+	* improve sanitation of symlinks, to support more complex link targets
 	* add DHT routing table affinity for BEP 42 nodes
 	* add torrent_info constructor overloads to control torrent file limits
 	* feature to disable DHT, PEX and LSD per torrent

include/libtorrent/aux_/path.hpp

@@ -139,6 +139,7 @@ namespace libtorrent {
 	// split out a path segment from the left side or right side
 	TORRENT_EXTRA_EXPORT std::pair<string_view, string_view> rsplit_path(string_view p);
 	TORRENT_EXTRA_EXPORT std::pair<string_view, string_view> lsplit_path(string_view p);
+	TORRENT_EXTRA_EXPORT std::pair<string_view, string_view> lsplit_path(string_view p, std::size_t pos);
 
 	TORRENT_EXTRA_EXPORT std::string extension(std::string const& f);
 	TORRENT_EXTRA_EXPORT std::string remove_extension(std::string const& f);

include/libtorrent/file_storage.hpp

@@ -532,6 +532,7 @@ namespace libtorrent {
 
 	private:
 
+		std::string internal_file_path(file_index_t index) const;
 		file_index_t last_file() const noexcept;
 
 		int get_or_add_path(string_view path);

include/libtorrent/string_view.hpp

@@ -39,17 +39,52 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #if BOOST_VERSION < 106100
 #include <boost/utility/string_ref.hpp>
+#include <cstring> // for strchr
 namespace libtorrent {
 
 using string_view = boost::string_ref;
 using wstring_view = boost::wstring_ref;
+
+inline string_view::size_type find_first_of(string_view const v, char const c
+	, string_view::size_type pos)
+{
+	while (pos < v.size())
+	{
+		if (v[pos] == c) return pos;
+		++pos;
+	}
+	return string_view::npos;
+}
+
+inline string_view::size_type find_first_of(string_view const v, char const* c
+	, string_view::size_type pos)
+{
+	while (pos < v.size())
+	{
+		if (std::strchr(c, v[pos]) != nullptr) return pos;
+		++pos;
+	}
+	return string_view::npos;
+}
 }
 #else
 #include <boost/utility/string_view.hpp>
 namespace libtorrent {
 
 using string_view = boost::string_view;
 using wstring_view = boost::wstring_view;
+
+inline string_view::size_type find_first_of(string_view const v, char const c
+	, string_view::size_type pos)
+{
+	return v.find_first_of(c, pos);
+}
+
+inline string_view::size_type find_first_of(string_view const v, char const* c
+	, string_view::size_type pos)
+{
+	return v.find_first_of(c, pos);
+}
 }
 #endif
 

src/file_storage.cpp

@@ -44,6 +44,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <cstdio>
 #include <algorithm>
 #include <functional>
+#include <set>
 
 #if defined(TORRENT_WINDOWS) || defined(TORRENT_OS2)
 #define TORRENT_SEPARATOR '\\'
@@ -657,7 +658,16 @@ namespace {
 		TORRENT_ASSERT_PRECOND(index >= file_index_t(0) && index < end_file());
 		internal_file_entry const& fe = m_files[index];
 		TORRENT_ASSERT(fe.symlink_index < int(m_symlinks.size()));
-		return m_symlinks[fe.symlink_index];
+
+		auto const& link = m_symlinks[fe.symlink_index];
+
+		// TODO: 3 this is a hack to retain ABI compatibility with 1.2.1
+		// in next major release, make this return by value
+		static std::string ret;
+		ret.reserve(m_name.size() + link.size() + 1);
+		ret.assign(m_name);
+		append_path(ret, link);
+		return ret;
 	}
 
 	std::time_t file_storage::mtime(file_index_t const index) const
@@ -818,6 +828,26 @@ namespace {
 		return ret;
 	}
 
+	std::string file_storage::internal_file_path(file_index_t const index) const
+	{
+		TORRENT_ASSERT_PRECOND(index >= file_index_t(0) && index < end_file());
+		internal_file_entry const& fe = m_files[index];
+
+		if (fe.path_index >= 0)
+		{
+			std::string ret;
+			std::string const& p = m_paths[fe.path_index];
+			ret.reserve(p.size() + fe.filename().size() + 2);
+			append_path(ret, p);
+			append_path(ret, fe.filename());
+			return ret;
+		}
+		else
+		{
+			return fe.filename().to_string();
+		}
+	}
+
 	string_view file_storage::file_name(file_index_t const index) const
 	{
 		TORRENT_ASSERT_PRECOND(index >= file_index_t(0) && index < end_file());
@@ -1112,13 +1142,26 @@ namespace {
 		std::unordered_map<std::string, file_index_t> file_map;
 		bool file_map_initialized = false;
 
+		// lazily instantiated set of all valid directories a symlink may point to
+		// TODO: in C++17 this could be string_view
+		std::unordered_set<std::string> dir_map;
+		bool dir_map_initialized = false;
+
+		// symbolic links that points to directories
+		std::unordered_map<std::string, std::string> dir_links;
+
+		// we validate symlinks in (potentially) 2 passes over the files.
+		// remaining symlinks to validate after the first pass
+		std::vector<file_index_t> symlinks_to_validate;
+
 		for (auto const i : file_range())
 		{
 			if (!(file_flags(i) & file_storage::flag_symlink)) continue;
 
 			if (!file_map_initialized)
 			{
-				for (auto const j : file_range()) file_map[file_path(j)] = j;
+				for (auto const j : file_range())
+					file_map.insert({internal_file_path(j), j});
 				file_map_initialized = true;
 			}
 
@@ -1128,54 +1171,145 @@ namespace {
 			// symlink targets are only allowed to point to files or directories in
 			// this torrent.
 			{
-				std::string target = symlink(i);
+				std::string target = m_symlinks[fe.symlink_index];
 
-				// if it points to a directory, that's OK
-				auto it = std::find(m_paths.begin(), m_paths.end(), target);
-				if (it != m_paths.end())
+				if (is_complete(target))
 				{
-					m_symlinks[fe.symlink_index] = combine_path(name(), *it);
+					// a symlink target is not allowed to be an absolute path, ever
+					// this symlink is invalid, make it point to itself
+					m_symlinks[fe.symlink_index] = internal_file_path(i);
 					continue;
 				}
 
-				target = combine_path(name(), target);
+				auto const iter = file_map.find(target);
+				if (iter != file_map.end())
+				{
+					m_symlinks[fe.symlink_index] = target;
+					if (file_flags(iter->second) & file_storage::flag_symlink)
+					{
+						// we don't know whether this symlink is a file or a
+						// directory, so make the conservative assumption that it's a
+						// directory
+						dir_links[internal_file_path(i)] = target;
+					}
+					continue;
+				}
 
-				auto const idx = file_map.find(target);
-				if (idx != file_map.end())
+				// it may point to a directory that doesn't have any files (but only
+				// other directories), in which case it won't show up in m_paths
+				if (!dir_map_initialized)
 				{
+					for (auto const& p : m_paths)
+						for (string_view pv = p; !pv.empty(); pv = rsplit_path(pv).first)
+							dir_map.insert(pv.to_string());
+					dir_map_initialized = true;
+				}
+
+				if (dir_map.count(target))
+				{
+					// it points to a sub directory within the torrent, that's OK
 					m_symlinks[fe.symlink_index] = target;
+					dir_links[internal_file_path(i)] = target;
 					continue;
 				}
-			}
 
-			// this symlink target points to a file that's not part of this torrent
-			// file structure. That's not allowed by the spec.
+			}
 
 			// for backwards compatibility, allow paths relative to the link as
 			// well
 			if (fe.path_index >= 0)
 			{
 				std::string target = m_paths[fe.path_index];
-				append_path(target, symlink(i));
+				append_path(target, m_symlinks[fe.symlink_index]);
 				// if it points to a directory, that's OK
-				auto it = std::find(m_paths.begin(), m_paths.end(), target);
+				auto const it = std::find(m_paths.begin(), m_paths.end(), target);
 				if (it != m_paths.end())
 				{
-					m_symlinks[fe.symlink_index] = combine_path(name(), *it);
+					m_symlinks[fe.symlink_index] = *it;
+					dir_links[internal_file_path(i)] = *it;
 					continue;
 				}
 
-				target = combine_path(name(), target);
-				auto const idx = file_map.find(target);
-				if (idx != file_map.end())
+				if (dir_map.count(target))
 				{
+					// it points to a sub directory within the torrent, that's OK
 					m_symlinks[fe.symlink_index] = target;
+					dir_links[internal_file_path(i)] = target;
+					continue;
+				}
+
+				auto const iter = file_map.find(target);
+				if (iter != file_map.end())
+				{
+					m_symlinks[fe.symlink_index] = target;
+					if (file_flags(iter->second) & file_storage::flag_symlink)
+					{
+						// we don't know whether this symlink is a file or a
+						// directory, so make the conservative assumption that it's a
+						// directory
+						dir_links[internal_file_path(i)] = target;
+					}
 					continue;
 				}
 			}
 
+			// we don't know whether this symlink is a file or a
+			// directory, so make the conservative assumption that it's a
+			// directory
+			dir_links[internal_file_path(i)] = m_symlinks[fe.symlink_index];
+			symlinks_to_validate.push_back(i);
+		}
+
+		// in case there were some "complex" symlinks, we nee a second pass to
+		// validate those. For example, symlinks whose target rely on other
+		// symlinks
+		for (auto const i : symlinks_to_validate)
+		{
+			internal_file_entry const& fe = m_files[i];
+			TORRENT_ASSERT(fe.symlink_index < int(m_symlinks.size()));
+
+			std::string target = m_symlinks[fe.symlink_index];
+
+			// to avoid getting stuck in an infinite loop, we only allow traversing
+			// a symlink once
+			std::set<std::string> traversed;
+
+			// this is where we check every path element for existence. If it's not
+			// among the concrete paths, it may be a symlink, which is also OK
+			// note that we won't iterate through this for the last step, where the
+			// filename is included. The filename is validated after the loop
+			for (string_view branch = lsplit_path(target).first;
+				branch.size() < target.size();
+				branch = lsplit_path(target, branch.size() + 1).first)
+			{
+				// this is a concrete directory
+				if (dir_map.count(branch.to_string())) continue;
+
+				auto const iter = dir_links.find(branch.to_string());
+				if (iter == dir_links.end()) goto failed;
+				if (traversed.count(branch.to_string())) goto failed;
+				traversed.insert(branch.to_string());
+
+				// this path element is a symlink. substitute the branch so far by
+				// the link target
+				target = combine_path(iter->second, target.substr(branch.size() + 1));
+
+				// start over with the new (concrete) path
+				branch = {};
+			}
+
+			// the final (resolved) target must be a valid file
+			// or directory
+			if (file_map.count(target) == 0
+				&& dir_map.count(target) == 0) goto failed;
+
+			// this is OK
+			continue;
+
+failed:
+
 			// this symlink is invalid, make it point to itself
-			m_symlinks[fe.symlink_index] = file_path(i);
+			m_symlinks[fe.symlink_index] = internal_file_path(i);
 		}
 	}
 

src/path.cpp

@@ -910,6 +910,25 @@ namespace {
 		return { p.substr(0, sep), p.substr(sep + 1) };
 	}
 
+	std::pair<string_view, string_view> lsplit_path(string_view p, std::size_t pos)
+	{
+		if (p.empty()) return {{}, {}};
+		// for absolute paths, skip the initial "/"
+		if (p.front() == TORRENT_SEPARATOR_CHAR
+#if defined(TORRENT_WINDOWS) || defined(TORRENT_OS2)
+			|| p.front() == '/'
+#endif
+		)
+		{ p.remove_prefix(1); if (pos > 0) --pos; }
+#if defined(TORRENT_WINDOWS) || defined(TORRENT_OS2)
+		auto const sep = find_first_of(p, "/\\", std::string::size_type(pos));
+#else
+		auto const sep = find_first_of(p, TORRENT_SEPARATOR_CHAR, std::string::size_type(pos));
+#endif
+		if (sep == string_view::npos) return {p, {}};
+		return { p.substr(0, sep), p.substr(sep + 1) };
+	}
+
 	std::string complete(string_view f)
 	{
 		if (is_complete(f)) return f.to_string();

test/Makefile.am

@@ -120,7 +120,8 @@ TEST_TORRENTS = \
   url_seed_multi_space.torrent \
   url_seed_multi_space_nolist.torrent \
   url_seed_multi_single_file.torrent \
-  whitespace_url.torrent
+  whitespace_url.torrent \
+  overlapping_symlinks.torrent
 
 MUTABLE_TEST_TORRENTS = \
   test1.torrent \

test/test_file.cpp

@@ -317,6 +317,42 @@ TORRENT_TEST(split_path)
 	TEST_CHECK(rsplit_path("") == r("", ""));
 }
 
+TORRENT_TEST(split_path_pos)
+{
+	using r = std::pair<string_view, string_view>;
+
+#ifdef TORRENT_WINDOWS
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 0) == r("b", "c\\d"));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 1) == r("b", "c\\d"));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 2) == r("b", "c\\d"));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 3) == r("b\\c", "d"));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 4) == r("b\\c", "d"));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 5) == r("b\\c\\d", ""));
+	TEST_CHECK(lsplit_path("\\b\\c\\d", 6) == r("b\\c\\d", ""));
+
+	TEST_CHECK(lsplit_path("b\\c\\d", 0) == r("b", "c\\d"));
+	TEST_CHECK(lsplit_path("b\\c\\d", 1) == r("b", "c\\d"));
+	TEST_CHECK(lsplit_path("b\\c\\d", 2) == r("b\\c", "d"));
+	TEST_CHECK(lsplit_path("b\\c\\d", 3) == r("b\\c", "d"));
+	TEST_CHECK(lsplit_path("b\\c\\d", 4) == r("b\\c\\d", ""));
+	TEST_CHECK(lsplit_path("b\\c\\d", 5) == r("b\\c\\d", ""));
+#endif
+	TEST_CHECK(lsplit_path("/b/c/d", 0) == r("b", "c/d"));
+	TEST_CHECK(lsplit_path("/b/c/d", 1) == r("b", "c/d"));
+	TEST_CHECK(lsplit_path("/b/c/d", 2) == r("b", "c/d"));
+	TEST_CHECK(lsplit_path("/b/c/d", 3) == r("b/c", "d"));
+	TEST_CHECK(lsplit_path("/b/c/d", 4) == r("b/c", "d"));
+	TEST_CHECK(lsplit_path("/b/c/d", 5) == r("b/c/d", ""));
+	TEST_CHECK(lsplit_path("/b/c/d", 6) == r("b/c/d", ""));
+
+	TEST_CHECK(lsplit_path("b/c/d", 0) == r("b", "c/d"));
+	TEST_CHECK(lsplit_path("b/c/d", 1) == r("b", "c/d"));
+	TEST_CHECK(lsplit_path("b/c/d", 2) == r("b/c", "d"));
+	TEST_CHECK(lsplit_path("b/c/d", 3) == r("b/c", "d"));
+	TEST_CHECK(lsplit_path("b/c/d", 4) == r("b/c/d", ""));
+	TEST_CHECK(lsplit_path("b/c/d", 5) == r("b/c/d", ""));
+}
+
 // file class
 TORRENT_TEST(file)
 {