fix handling of invalid filenames

ChangeLog

@@ -1,3 +1,6 @@
+2.0.12
+
+	* fix handling of invalid filenames
 
 2.0.11 released
 

Makefile

@@ -1050,9 +1050,11 @@ TEST_TORRENTS = \
   whitespace_url.torrent \
   v2.torrent \
   v2_empty_file.torrent \
+  v2_empty_filename.torrent \
   v2_multipiece_file.torrent \
   v2_only.torrent \
   v2_invalid_filename.torrent \
+  v2_invalid_filename2.torrent \
   v2_mismatching_metadata.torrent \
   v2_no_power2_piece.torrent \
   v2_invalid_file.torrent \

include/libtorrent/torrent_info.hpp

@@ -72,7 +72,7 @@ namespace aux {
 
 	// internal, exposed for the unit test
 	TORRENT_EXTRA_EXPORT void sanitize_append_path_element(std::string& path
-		, string_view element);
+		, string_view element, bool force_element = false);
 	TORRENT_EXTRA_EXPORT bool verify_encoding(std::string& target);
 
 	struct internal_drained_state

src/torrent_info.cpp

@@ -167,9 +167,14 @@ namespace aux {
 		return valid_encoding;
 	}
 
-	void sanitize_append_path_element(std::string& path, string_view element)
+	// it's important that every call adds a path element to the path, even if
+	// the name is invalid. It can never be empty. Empty files have special
+	// meaning in v2 torrents (it means the previous path element was the
+	// filename). Also, If we're adding the torrent name as the first path
+	// element, in a multi-file torrent, we must have a directory name.
+	void sanitize_append_path_element(std::string& path, string_view element, bool const force_element)
 	{
-		if (element.size() == 1 && element[0] == '.') return;
+		if (element.size() == 1 && element[0] == '.' && !force_element) return;
 
 #ifdef TORRENT_WINDOWS
 #define TORRENT_SEPARATOR '\\'
@@ -296,8 +301,17 @@ namespace aux {
 
 		if (added == num_dots && added <= 2)
 		{
-			// revert everything
-			path.erase(path.end() - added - added_separator, path.end());
+			if (force_element)
+			{
+				// revert the invalid filename and replace it with an underscore
+				path.erase(path.end() - added, path.end());
+				path += "_";
+			}
+			else
+			{
+				// revert everything
+				path.erase(path.end() - added - added_separator, path.end());
+			}
 			return;
 		}
 
@@ -311,7 +325,11 @@ namespace aux {
 			TORRENT_ASSERT(added >= 0);
 		}
 
-		if (added == 0 && added_separator)
+		if (force_element && added == 0)
+		{
+			path += "_";
+		}
+		else if (added == 0 && added_separator)
 		{
 			// remove the separator added at the beginning
 			path.erase(path.end() - 1);
@@ -445,7 +463,7 @@ namespace {
 	// torrent, in which case it's empty.
 	bool extract_single_file(bdecode_node const& dict, file_storage& files
 		, std::string const& root_dir, std::ptrdiff_t const info_offset
-		, char const* info_buffer, bool top_level, error_code& ec)
+		, char const* info_buffer, bool const top_level, error_code& ec)
 	{
 		if (dict.type() != bdecode_node::dict_t) return false;
 
@@ -505,7 +523,6 @@ namespace {
 			if (p && p.list_size() > 0)
 			{
 				std::size_t const preallocate = path.size() + std::size_t(path_length(p, ec));
-				std::size_t const orig_path_len = path.size();
 				if (ec) return false;
 				path.reserve(preallocate);
 
@@ -519,15 +536,7 @@ namespace {
 						while (!filename.empty() && filename.front() == TORRENT_SEPARATOR)
 							filename.remove_prefix(1);
 					}
-					aux::sanitize_append_path_element(path, e.string_value());
-				}
-
-				// if all path elements were sanitized away, we need to use another
-				// name instead
-				if (path.size() == orig_path_len)
-				{
-					path += TORRENT_SEPARATOR;
-					path += "_";
+					aux::sanitize_append_path_element(path, e.string_value(), true);
 				}
 			}
 			else if (file_flags & file_storage::flag_pad_file)
@@ -628,7 +637,7 @@ namespace {
 			bool const single_file = leaf_node && !has_files && tree.dict_size() == 1;
 
 			std::string path = single_file ? std::string() : root_dir;
-			aux::sanitize_append_path_element(path, filename);
+			aux::sanitize_append_path_element(path, filename, true);
 
 			if (leaf_node)
 			{

test/test_torrent_info.cpp

@@ -120,12 +120,12 @@ static test_torrent_t const test_torrents[] =
 	{ "single_multi_file.torrent" },
 	{ "slash_path.torrent", [](torrent_info const* ti) {
 			TEST_EQUAL(ti->num_files(), 1);
-			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "temp" SEPARATOR "bar");
+			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "temp" SEPARATOR "_" SEPARATOR "_" SEPARATOR "bar");
 		}
 	},
 	{ "slash_path2.torrent", [](torrent_info const* ti) {
 			TEST_EQUAL(ti->num_files(), 1);
-			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "temp" SEPARATOR "abc....def" SEPARATOR "bar");
+			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "temp" SEPARATOR "abc....def" SEPARATOR "_" SEPARATOR "bar");
 		}
 	},
 	{ "slash_path3.torrent", [](torrent_info const* ti) {
@@ -317,6 +317,13 @@ static test_torrent_t const test_torrents[] =
 			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "_estMB"_sv);
 		}
 	},
+	{ "v2_invalid_filename2.torrent", [](torrent_info const* ti) {
+			TEST_EQUAL(ti->num_files(), 3);
+			TEST_EQUAL(ti->files().file_path(file_index_t{0}), "test" SEPARATOR "_"_sv);
+			TEST_EQUAL(ti->files().file_path(file_index_t{1}), "test" SEPARATOR "_.1"_sv);
+			TEST_EQUAL(ti->files().file_path(file_index_t{2}), "test" SEPARATOR "stress_test2"_sv);
+		}
+	},
 	{ "v2_multiple_files.torrent", [](torrent_info* ti) {
 			TEST_EQUAL(ti->v2_piece_hashes_verified(), true);
 			TEST_EQUAL(ti->num_files(), 5);
@@ -452,6 +459,7 @@ test_failing_torrent_t test_error_torrents[] =
 	{ "v2_invalid_pad_file.torrent", errors::torrent_invalid_pad_file},
 	{ "v2_zero_root.torrent", errors::torrent_missing_pieces_root},
 	{ "v2_zero_root_small.torrent", errors::torrent_missing_pieces_root},
+	{ "v2_empty_filename.torrent", errors::torrent_file_parse_failed},
 };
 
 } // anonymous namespace
@@ -863,6 +871,194 @@ TORRENT_TEST(sanitize_path)
 	TEST_EQUAL(path, "foobar");
 }
 
+TORRENT_TEST(sanitize_path_force)
+{
+	using lt::aux::sanitize_append_path_element;
+	std::string path;
+	sanitize_append_path_element(path, "\0\0\xed\0\x80", true);
+	TEST_EQUAL(path, "_");
+
+	path.clear();
+	sanitize_append_path_element(path, "/a/", true);
+	sanitize_append_path_element(path, "b", true);
+	sanitize_append_path_element(path, "c", true);
+	TEST_EQUAL(path, "a" SEPARATOR "b" SEPARATOR "c");
+
+	path.clear();
+	sanitize_append_path_element(path, "a...b", true);
+	TEST_EQUAL(path, "a...b");
+
+	path.clear();
+	sanitize_append_path_element(path, "a", true);
+	sanitize_append_path_element(path, "..", true);
+	sanitize_append_path_element(path, "c", true);
+	TEST_EQUAL(path, "a" SEPARATOR "_" SEPARATOR "c");
+
+	path.clear();
+	sanitize_append_path_element(path, "a", true);
+	sanitize_append_path_element(path, "..", true);
+	TEST_EQUAL(path, "a" SEPARATOR "_");
+
+	path.clear();
+	sanitize_append_path_element(path, "/..", true);
+	sanitize_append_path_element(path, ".", true);
+	sanitize_append_path_element(path, "c", true);
+	TEST_EQUAL(path, "_" SEPARATOR "_" SEPARATOR "c");
+
+	path.clear();
+	sanitize_append_path_element(path, "dev:", true);
+#ifdef TORRENT_WINDOWS
+	TEST_EQUAL(path, "dev_");
+#else
+	TEST_EQUAL(path, "dev:");
+#endif
+
+	path.clear();
+	sanitize_append_path_element(path, "c:", true);
+	sanitize_append_path_element(path, "b", true);
+#ifdef TORRENT_WINDOWS
+	TEST_EQUAL(path, "c_" SEPARATOR "b");
+#else
+	TEST_EQUAL(path, "c:" SEPARATOR "b");
+#endif
+
+	path.clear();
+	sanitize_append_path_element(path, "c:", true);
+	sanitize_append_path_element(path, ".", true);
+	sanitize_append_path_element(path, "c", true);
+#ifdef TORRENT_WINDOWS
+	TEST_EQUAL(path, "c_" SEPARATOR "_" SEPARATOR "c");
+#else
+	TEST_EQUAL(path, "c:" SEPARATOR "_" SEPARATOR "c");
+#endif
+
+	path.clear();
+	sanitize_append_path_element(path, "\\c", true);
+	sanitize_append_path_element(path, ".", true);
+	sanitize_append_path_element(path, "c", true);
+	TEST_EQUAL(path, "c" SEPARATOR "_" SEPARATOR "c");
+
+	path.clear();
+	sanitize_append_path_element(path, "\b", true);
+	TEST_EQUAL(path, "_");
+
+	path.clear();
+	sanitize_append_path_element(path, "\b", true);
+	sanitize_append_path_element(path, "filename", true);
+	TEST_EQUAL(path, "_" SEPARATOR "filename");
+
+	path.clear();
+	sanitize_append_path_element(path, "filename", true);
+	sanitize_append_path_element(path, "\b", true);
+	TEST_EQUAL(path, "filename" SEPARATOR "_");
+
+	path.clear();
+	sanitize_append_path_element(path, "abc", true);
+	sanitize_append_path_element(path, "", true);
+	TEST_EQUAL(path, "abc" SEPARATOR "_");
+
+	path.clear();
+	sanitize_append_path_element(path, "abc", true);
+	sanitize_append_path_element(path, "   ", true);
+#ifdef TORRENT_WINDOWS
+	TEST_EQUAL(path, "abc" SEPARATOR "_");
+#else
+	TEST_EQUAL(path, "abc" SEPARATOR "   ");
+#endif
+
+	path.clear();
+	sanitize_append_path_element(path, "", true);
+	sanitize_append_path_element(path, "abc", true);
+	TEST_EQUAL(path, "_" SEPARATOR "abc");
+
+	path.clear();
+	sanitize_append_path_element(path, "\b?filename=4", true);
+#ifdef TORRENT_WINDOWS
+	TEST_EQUAL(path, "__filename=4");
+#else
+	TEST_EQUAL(path, "_?filename=4");
+#endif
+
+	path.clear();
+	sanitize_append_path_element(path, "filename=4", true);
+	TEST_EQUAL(path, "filename=4");
+
+	// valid 2-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xc2\xa1", true);
+	TEST_EQUAL(path, "filename\xc2\xa1");
+
+	// truncated 2-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xc2", true);
+	TEST_EQUAL(path, "filename_");
+
+	// valid 3-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xe2\x9f\xb9", true);
+	TEST_EQUAL(path, "filename\xe2\x9f\xb9");
+
+	// truncated 3-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xe2\x9f", true);
+	TEST_EQUAL(path, "filename_");
+
+	// truncated 3-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xe2", true);
+	TEST_EQUAL(path, "filename_");
+
+	// valid 4-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xf0\x9f\x92\x88", true);
+	TEST_EQUAL(path, "filename\xf0\x9f\x92\x88");
+
+	// truncated 4-byte sequence
+	path.clear();
+	sanitize_append_path_element(path, "filename\xf0\x9f\x92", true);
+	TEST_EQUAL(path, "filename_");
+
+	// 5-byte utf-8 sequence (not allowed)
+	path.clear();
+	sanitize_append_path_element(path, "filename\xf8\x9f\x9f\x9f\x9f" "foobar", true);
+	TEST_EQUAL(path, "filename_foobar");
+
+	// redundant (overlong) 2-byte sequence
+	// ascii code 0x2e encoded with a leading 0
+	path.clear();
+	sanitize_append_path_element(path, "filename\xc0\xae", true);
+	TEST_EQUAL(path, "filename_");
+
+	// redundant (overlong) 3-byte sequence
+	// ascii code 0x2e encoded with two leading 0s
+	path.clear();
+	sanitize_append_path_element(path, "filename\xe0\x80\xae", true);
+	TEST_EQUAL(path, "filename_");
+
+	// redundant (overlong) 4-byte sequence
+	// ascii code 0x2e encoded with three leading 0s
+	path.clear();
+	sanitize_append_path_element(path, "filename\xf0\x80\x80\xae", true);
+	TEST_EQUAL(path, "filename_");
+
+	// a filename where every character is filtered is not replaced by an understcore
+	path.clear();
+	sanitize_append_path_element(path, "//\\", true);
+	TEST_EQUAL(path, "_");
+
+	// make sure suspicious unicode characters are filtered out
+	path.clear();
+	// that's utf-8 for U+200e LEFT-TO-RIGHT MARK
+	sanitize_append_path_element(path, "foo\xe2\x80\x8e" "bar", true);
+	TEST_EQUAL(path, "foobar");
+
+	// make sure suspicious unicode characters are filtered out
+	path.clear();
+	// that's utf-8 for U+202b RIGHT-TO-LEFT EMBEDDING
+	sanitize_append_path_element(path, "foo\xe2\x80\xab" "bar", true);
+	TEST_EQUAL(path, "foobar");
+}
+
 TORRENT_TEST(sanitize_path_zeroes)
 {
 	using lt::aux::sanitize_append_path_element;
@@ -993,7 +1189,6 @@ void sanity_check(std::shared_ptr<torrent_info> const& ti)
 TORRENT_TEST(parse_torrents)
 {
 	// test torrent parsing
-
 	entry info;
 	info["pieces"] = "aaaaaaaaaaaaaaaaaaaa";
 	info["name.utf-8"] = "test1";
@@ -1032,7 +1227,6 @@ TORRENT_TEST(parse_torrents)
 	torrent_info ti3(buf, from_span);
 	std::cout << ti3.name() << std::endl;
 	TEST_EQUAL(ti3.name(), "test2..test3.......test4");
-
 	std::string root_dir = parent_path(current_working_directory());
 	for (auto const& t : test_torrents)
 	{
@@ -1041,8 +1235,8 @@ TORRENT_TEST(parse_torrents)
 			, t.file);
 		error_code ec;
 		auto ti = std::make_shared<torrent_info>(filename, ec);
-		TEST_CHECK(!ec);
 		if (ec) std::printf(" -> failed %s\n", ec.message().c_str());
+		TEST_CHECK(!ec);
 		sanity_check(ti);
 
 		add_torrent_params atp = load_torrent_file(filename);

test/test_torrents/v2_empty_filename.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1554199319e4:infod9:file treed0:d0:d6:lengthi1048576e11:pieces root32:m�$��80y߬ΐ��O0�h���?����=ee1:#d0:d6:lengthi1048576e11:pieces root32:[�f��0�dLj<�_�7�}5BJ=P��U�;eee12:meta versioni2e4:name4:test12:piece lengthi1048576eeee

test/test_torrents/v2_invalid_filename2.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1554199319e4:infod9:file treed2://d0:d6:lengthi1048576e11:pieces root32:m�$��80y߬ΐ��O0�h���?����=ee1:\d0:d6:lengthi1048576e11:pieces root32:[�f��0�dLj<�_�7�}5BJ=P��U�;ee12:stress_test2d0:d6:lengthi1048576e11:pieces root32:�kܣ٧b�ӻV�zx������Z*A�D���%�eee12:meta versioni2e4:name4:test12:piece lengthi1048576eee