fix: possible heap-use-after-free with magnet links (#6815)

* fix: queue torrent verification as soon as metadata complete

* fix: avoid heap-use-after-free in `tr_peerMsgsImpl::process_peer_message()`

Details: https://github.com/transmission/transmission/pull/6383#discussion_r1429202253

* code review: move `tr_torrent::do_magnet_idle_work()` to private

* code review: `std::deque::empty()` is not `constexpr`

* fix: test

libtransmission/torrent-magnet.cc

@@ -68,6 +68,15 @@ tr_metadata_download::tr_metadata_download(std::string_view log_name, int64_t co
     create_all_needed(n);
 }
 
+void tr_torrent::do_magnet_idle_work()
+{
+    if (auto& m = metadata_download_; m && m->is_complete())
+    {
+        tr_logAddDebugTor(this, fmt::format("we now have all the metainfo!"));
+        on_have_all_metainfo();
+    }
+}
+
 void tr_torrent::maybe_start_metadata_transfer(int64_t const size) noexcept
 {
     if (has_metainfo() || metadata_download_)
@@ -262,20 +271,20 @@ void tr_torrent::on_have_all_metainfo()
     m.reset();
 }
 
-bool tr_metadata_download::set_metadata_piece(int64_t const piece, void const* const data, size_t const len)
+void tr_metadata_download::set_metadata_piece(int64_t const piece, void const* const data, size_t const len)
 {
     TR_ASSERT(data != nullptr);
 
     // sanity test: is `piece` in range?
     if (piece < 0 || piece >= piece_count_)
     {
-        return false;
+        return;
     }
 
     // sanity test: is `len` the right size?
     if (get_piece_length(piece) != len)
     {
-        return false;
+        return;
     }
 
     // do we need this piece?
@@ -286,7 +295,7 @@ bool tr_metadata_download::set_metadata_piece(int64_t const piece, void const* c
         [piece](auto const& item) { return item.piece == piece; });
     if (iter == std::end(needed))
     {
-        return false;
+        return;
     }
 
     auto const offset = piece * MetadataPieceSize;
@@ -294,8 +303,6 @@ bool tr_metadata_download::set_metadata_piece(int64_t const piece, void const* c
 
     needed.erase(iter);
     tr_logAddDebugMagnet(this, fmt::format("saving metainfo piece {}... {} remain", piece, std::size(needed)));
-
-    return std::empty(needed);
 }
 
 void tr_torrent::set_metadata_piece(int64_t const piece, void const* const data, size_t const len)
@@ -304,10 +311,9 @@ void tr_torrent::set_metadata_piece(int64_t const piece, void const* const data,
 
     tr_logAddDebugTor(this, fmt::format("got metadata piece {} of {} bytes", piece, len));
 
-    if (auto& m = metadata_download_; m && m->set_metadata_piece(piece, data, len))
+    if (auto& m = metadata_download_)
     {
-        tr_logAddDebugTor(this, fmt::format("we now have all the metainfo!"));
+        m->set_metadata_piece(piece, data, len);
-        on_have_all_metainfo();
     }
 }
 

libtransmission/torrent-magnet.h

@@ -39,7 +39,12 @@ public:
         return size > 0 && size <= std::numeric_limits<int>::max();
     }
 
-    bool set_metadata_piece(int64_t piece, void const* data, size_t len);
+    [[nodiscard]] auto is_complete() const noexcept
+    {
+        return std::empty(pieces_needed_);
+    }
+
+    void set_metadata_piece(int64_t piece, void const* data, size_t len);
 
     [[nodiscard]] std::optional<int64_t> get_next_metadata_request(time_t now) noexcept;
 

libtransmission/torrent.cc

@@ -899,7 +899,7 @@ void tr_torrent::on_metainfo_completed()
         // Potentially, we are in `tr_torrent::init`,
         // and we don't want any file created before `tr_torrent::start`
         // so we Verify but we don't Create files.
-        session->queue_session_thread(tr_torrentVerify, this);
+        tr_torrentVerify(this);
     }
     else
     {

libtransmission/torrent.h

@@ -887,6 +887,8 @@ struct tr_torrent
 
     void do_idle_work()
     {
+        do_magnet_idle_work();
+
         if (needs_completeness_check_)
         {
             needs_completeness_check_ = false;
@@ -1244,6 +1246,7 @@ private:
     void create_empty_files() const;
     void recheck_completeness();
 
+    void do_magnet_idle_work();
     [[nodiscard]] bool use_new_metainfo(tr_error* error);
 
     void set_location_in_session_thread(std::string_view path, bool move_from_old_path, int volatile* setme_state);

tests/libtransmission/torrent-magnet-test.cc

@@ -85,6 +85,7 @@ TEST_F(TorrentMagnetTest, setMetadataPiece)
 
             tor->maybe_start_metadata_transfer(metainfo_size);
             tor->set_metadata_piece(0, std::data(metainfo_benc), metainfo_size);
+            tor->do_idle_work();
             EXPECT_TRUE(tor->has_metainfo());
             EXPECT_EQ(tor->info_dict_size(), metainfo_size);
             EXPECT_EQ(tor->get_metadata_percent(), 1.0);