diff --git a/ChangeLog b/ChangeLog index aeae887d7..c372b760f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * fix handling of torrents with too large pieces * fixed division by zero in anti-leech choker * fixed bug in torrent_info::swap diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 6574b49f5..7a0f13006 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -95,7 +95,11 @@ namespace libtorrent { // the number of priority levels priority_levels = 8, // priority factor - prio_factor = 3 + prio_factor = 3, + // max blocks per piece + // there are counters in downloading_piece that only have 15 bits to + // count blocks per piece, that's restricting this + max_blocks_per_piece = (1 << 15) - 1 }; struct block_info diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 6d4b9c2eb..8a22bf7b8 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -1699,6 +1699,11 @@ namespace libtorrent { // millionths of completeness) std::uint32_t m_progress_ppm:20; + // set to true once init() completes successfully. This is important to + // track in case it fails and need to be retried if the client clears + // the torrent error + bool m_torrent_initialized:1; + #if TORRENT_USE_ASSERTS // set to true when torrent is start()ed. It may only be started once bool m_was_started = false; diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index bc6f177da..db88b7033 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -155,6 +155,10 @@ namespace libtorrent { #ifdef TORRENT_PICKER_LOG std::cerr << "[" << this << "] " << "piece_picker::resize()" << std::endl; #endif + + if (blocks_per_piece > max_blocks_per_piece) + throw system_error(errors::invalid_piece_size); + // allocate the piece_map to cover all pieces // and make them invalid (as if we don't have a single piece) m_piece_map.resize(total_num_pieces, piece_pos(0, 0)); @@ -191,9 +195,9 @@ namespace libtorrent { m_reverse_cursor > piece_index_t(0) && (i->have() || i->filtered()); ++i, --m_reverse_cursor); - m_blocks_per_piece = std::uint16_t(blocks_per_piece); - m_blocks_in_last_piece = std::uint16_t(blocks_in_last_piece); - if (m_blocks_in_last_piece == 0) m_blocks_in_last_piece = std::uint16_t(blocks_per_piece); + m_blocks_per_piece = aux::numeric_cast(blocks_per_piece); + m_blocks_in_last_piece = aux::numeric_cast(blocks_in_last_piece); + if (m_blocks_in_last_piece == 0) m_blocks_in_last_piece = aux::numeric_cast(blocks_per_piece); TORRENT_ASSERT(m_blocks_in_last_piece <= m_blocks_per_piece); } diff --git a/src/torrent.cpp b/src/torrent.cpp index db319d0fb..2045cbdf9 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -226,6 +226,7 @@ bool is_downloading_state(int const st) , m_inactive(false) , m_downloaded(0xffffff) , m_progress_ppm(0) + , m_torrent_initialized(false) { // we cannot log in the constructor, because it relies on shared_from_this // being initialized, which happens after the constructor returns. @@ -1776,6 +1777,15 @@ bool is_downloading_state(int const st) return; } + int const blocks_per_piece + = (m_torrent_file->piece_length() + default_block_size - 1) / default_block_size; + if (blocks_per_piece > piece_picker::max_blocks_per_piece) + { + set_error(errors::invalid_piece_size, torrent_status::error_file_none); + pause(); + return; + } + // --- MAPPED FILES --- file_storage const& fs = m_torrent_file->files(); if (m_add_torrent_params) @@ -1860,7 +1870,6 @@ bool is_downloading_state(int const st) TORRENT_ASSERT((pr.start & (block_size() - 1)) == 0); int block = block_size(); - int blocks_per_piece = m_torrent_file->piece_length() / block; piece_block pb(pr.piece, pr.start / block); for (; pr.length >= block; pr.length -= block, ++pb.block_index) { @@ -1970,6 +1979,8 @@ bool is_downloading_state(int const st) // this will remove the piece picker, if we're done with it maybe_done_flushing(); + + m_torrent_initialized = true; } bt_peer_connection* torrent::find_introducer(tcp::endpoint const& ep) const @@ -8183,7 +8194,7 @@ bool is_downloading_state(int const st) } #endif // if the error happened during initialization, try again now - if (!m_connections_initialized && valid_metadata()) init(); + if (!m_torrent_initialized && valid_metadata()) init(); if (!checking_files && should_check_files()) start_checking(); } diff --git a/test/test_torrent.cpp b/test/test_torrent.cpp index 1bf511a4b..1d0d4e779 100644 --- a/test/test_torrent.cpp +++ b/test/test_torrent.cpp @@ -166,11 +166,34 @@ void test_running_torrent(std::shared_ptr info, std::int64_t file_ TEST_CHECK(h.get_file_priorities() == prio); } +void test_large_piece_size(int const size) +{ + entry torrent; + entry& info = torrent["info"]; + info["pieces"] = "aaaaaaaaaaaaaaaaaaaa"; + info["name"] = "test"; + info["piece length"] = size; + info["length"] = size; + + std::vector buf; + bencode(std::back_inserter(buf), torrent); + add_torrent_params atp; + atp.ti = std::make_shared(buf, from_span); + atp.save_path = "."; + + lt::session ses; + auto h = ses.add_torrent(atp); + TEST_CHECK(h.status().errc == error_code(lt::errors::invalid_piece_size)); + h.clear_error(); + TEST_CHECK(h.status().errc == error_code(lt::errors::invalid_piece_size)); +} + } // anonymous namespace TORRENT_TEST(long_names) { - entry info; + entry torrent; + entry& info = torrent["info"]; info["pieces"] = "aaaaaaaaaaaaaaaaaaaa"; info["name"] = "slightly shorter name, it's kind of sad that people started " "the trend of incorrectly encoding the regular name field and then adding " @@ -180,14 +203,17 @@ TORRENT_TEST(long_names) "read this that particular bug should have been fixed"; info["piece length"] = 16 * 1024; info["length"] = 3245; - entry torrent; - torrent["info"] = info; std::vector buf; bencode(std::back_inserter(buf), torrent); - error_code ec; - auto ti = std::make_shared(buf, std::ref(ec), from_span); - TEST_CHECK(!ec); + auto ti = std::make_shared(buf, from_span); +} + +TORRENT_TEST(large_piece_size) +{ + test_large_piece_size(32768 * 16 * 1024); + test_large_piece_size(65536 * 16 * 1024); + test_large_piece_size(65537 * 16 * 1024); } TORRENT_TEST(total_wanted) @@ -202,8 +228,7 @@ TORRENT_TEST(total_wanted) lt::create_torrent t(fs, 1024); std::vector tmp; bencode(std::back_inserter(tmp), t.generate()); - error_code ec; - auto info = std::make_shared(tmp, std::ref(ec), from_span); + auto info = std::make_shared(tmp, from_span); settings_pack pack = settings(); pack.set_int(settings_pack::alert_mask, alert::storage_notification); @@ -243,9 +268,7 @@ TORRENT_TEST(added_peers) lt::create_torrent t(fs, 1024); std::vector tmp; bencode(std::back_inserter(tmp), t.generate()); - error_code ec; - auto info = std::make_shared( - tmp, std::ref(ec), from_span); + auto info = std::make_shared(tmp, from_span); settings_pack pack = settings(); pack.set_str(settings_pack::listen_interfaces, "0.0.0.0:48130"); @@ -253,11 +276,9 @@ TORRENT_TEST(added_peers) lt::session ses(pack); add_torrent_params p = parse_magnet_uri( - "magnet:?xt=urn:btih:abababababababababababababababababababab&x.pe=127.0.0.1:48081&x.pe=127.0.0.2:48082" - , ec); + "magnet:?xt=urn:btih:abababababababababababababababababababab&x.pe=127.0.0.1:48081&x.pe=127.0.0.2:48082"); p.ti = info; p.save_path = "."; - TEST_CHECK(!ec); torrent_handle h = ses.add_torrent(std::move(p)); @@ -324,8 +345,7 @@ TORRENT_TEST(torrent) std::vector tmp; std::back_insert_iterator> out(tmp); bencode(out, t.generate()); - error_code ec; - auto info = std::make_shared(tmp, std::ref(ec), from_span); + auto info = std::make_shared(tmp, from_span); test_running_torrent(info, 1024); } } @@ -367,13 +387,12 @@ TORRENT_TEST(duplicate_is_not_error) std::vector tmp; std::back_insert_iterator> out(tmp); bencode(out, t.generate()); - error_code ec; int called = 0; plugin_creator creator(called); add_torrent_params p; - p.ti = std::make_shared(tmp, std::ref(ec), from_span); + p.ti = std::make_shared(tmp, from_span); p.flags &= ~torrent_flags::paused; p.flags &= ~torrent_flags::auto_managed; p.flags &= ~torrent_flags::duplicate_is_error; @@ -394,13 +413,12 @@ TORRENT_TEST(duplicate_is_not_error) TORRENT_TEST(torrent_total_size_zero) { file_storage fs; - error_code ec; fs.add_file("test_torrent_dir2/tmp1", 0); TEST_CHECK(fs.num_files() == 1); TEST_CHECK(fs.total_size() == 0); - ec.clear(); + error_code ec; lt::create_torrent t1(fs); set_piece_hashes(t1, ".", ec); TEST_CHECK(ec); @@ -426,8 +444,7 @@ TORRENT_TEST(rename_file) std::vector tmp; std::back_insert_iterator> out(tmp); bencode(out, t.generate()); - error_code ec; - auto info = std::make_shared(tmp, std::ref(ec), from_span); + auto info = std::make_shared(tmp, from_span); TEST_EQUAL(info->files().file_path(file_index_t(0)), combine_path("test3","tmp1")); @@ -605,8 +622,7 @@ TORRENT_TEST(queue_paused) TORRENT_TEST(test_move_storage_no_metadata) { lt::session ses(settings()); - error_code ec; - add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec); + add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab"); p.save_path = "save_path"; torrent_handle h = ses.add_torrent(p); @@ -620,8 +636,7 @@ TORRENT_TEST(test_move_storage_no_metadata) TORRENT_TEST(test_have_piece_no_metadata) { lt::session ses(settings()); - error_code ec; - add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec); + add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab"); p.save_path = "save_path"; torrent_handle h = ses.add_torrent(p); @@ -650,8 +665,7 @@ TORRENT_TEST(test_have_piece_out_of_range) TORRENT_TEST(test_read_piece_no_metadata) { lt::session ses(settings()); - error_code ec; - add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec); + add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab"); p.save_path = "save_path"; torrent_handle h = ses.add_torrent(p);