fix: check full path when parsing http tracker responses (#7326)

* refactor: move path checking methods to base

* refactor: differentiate "empty string key" and "no key"

* fix: check full path when parsing http announce response

* fix: check full path when parsing http scrape response

* refactor: avoid copying when checking scrape info hash

* fixup! fix: check full path when parsing http scrape response

libtransmission/announcer-http.cc

@@ -346,33 +346,33 @@ void tr_announcerParseHttpAnnounceResponse(tr_announce_response& response, std::
 
         bool Int64(int64_t value, Context const& /*context*/) override
         {
-            if (auto const key = currentKey(); key == "interval")
+            if (pathIs("interval"sv))
             {
                 response_.interval = static_cast<int>(value);
             }
-            else if (key == "min interval"sv)
+            else if (pathIs("min interval"sv))
             {
                 response_.min_interval = static_cast<int>(value);
             }
-            else if (key == "complete"sv)
+            else if (pathIs("complete"sv))
             {
                 response_.seeders = value;
             }
-            else if (key == "incomplete"sv)
+            else if (pathIs("incomplete"sv))
             {
                 response_.leechers = value;
             }
-            else if (key == "downloaded"sv)
+            else if (pathIs("downloaded"sv))
             {
                 response_.downloads = value;
             }
-            else if (key == "port"sv)
+            else if (pathIs("peers"sv, ArrayKey, "port"sv))
             {
                 pex_.socket_address.port_.set_host(static_cast<uint16_t>(value));
             }
             else
             {
-                tr_logAddDebug(fmt::format("unexpected key '{}' int '{}'", key, value), log_name_);
+                tr_logAddDebug(fmt::format("unexpected path '{}' int '{}'", path(), value), log_name_);
             }
 
             return true;
@@ -380,45 +380,45 @@ void tr_announcerParseHttpAnnounceResponse(tr_announce_response& response, std::
 
         bool String(std::string_view value, Context const& /*context*/) override
         {
-            if (auto const key = currentKey(); key == "failure reason"sv)
+            if (pathIs("failure reason"sv))
             {
                 response_.errmsg = value;
             }
-            else if (key == "warning message"sv)
+            else if (pathIs("warning message"sv))
             {
                 response_.warning = value;
             }
-            else if (key == "tracker id"sv)
+            else if (pathIs("tracker id"sv))
             {
                 response_.tracker_id = value;
             }
-            else if (key == "peers"sv)
+            else if (pathIs("peers"sv))
             {
                 response_.pex = tr_pex::from_compact_ipv4(std::data(value), std::size(value), nullptr, 0);
             }
-            else if (key == "peers6"sv)
+            else if (pathIs("peers6"sv))
             {
                 response_.pex6 = tr_pex::from_compact_ipv6(std::data(value), std::size(value), nullptr, 0);
             }
-            else if (key == "ip")
+            else if (pathIs("peers"sv, ArrayKey, "ip"sv))
             {
                 if (auto const addr = tr_address::from_string(value); addr)
                 {
                     pex_.socket_address.address_ = *addr;
                 }
             }
-            else if (key == "peer id")
+            else if (pathIs("peers"sv, ArrayKey, "peer id"sv))
             {
                 // unused
             }
-            else if (key == "external ip"sv && std::size(value) == 4)
+            else if (pathIs("external ip"sv) && std::size(value) == 4)
             {
                 auto const [addr, out] = tr_address::from_compact_ipv4(reinterpret_cast<std::byte const*>(std::data(value)));
                 response_.external_ip = addr;
             }
             else
             {
-                tr_logAddDebug(fmt::format("unexpected key '{}' int '{}'", key, value), log_name_);
+                tr_logAddDebug(fmt::format("unexpected path '{}' int '{}'", path(), value), log_name_);
             }
 
             return true;
@@ -566,22 +566,34 @@ void tr_announcerParseHttpScrapeResponse(tr_scrape_response& response, std::stri
         {
             BasicHandler::Key(value, context);
 
-            if (auto needle = tr_sha1_digest_t{}; depth() == 2 && key(1) == "files"sv && std::size(value) == std::size(needle))
+            if (auto cur_depth = depth(); cur_depth < 2U || !pathStartsWith("files"sv))
             {
-                std::copy_n(reinterpret_cast<std::byte const*>(std::data(value)), std::size(value), std::data(needle));
-                auto const it = std::find_if(
-                    std::begin(response_.rows),
-                    std::end(response_.rows),
-                    [needle](auto const& row) { return row.info_hash == needle; });
-
-                if (it == std::end(response_.rows))
-                {
-                    row_.reset();
-                }
-                else
-                {
-                    row_ = std::distance(std::begin(response_.rows), it);
-                }
+                row_.reset();
+            }
+            else if (cur_depth > 2U)
+            {
+                // do nothing
+            }
+            else if (std::size(value) != std::tuple_size_v<tr_sha1_digest_t>)
+            {
+                row_.reset();
+            }
+            else if (auto const it = std::find_if(
+                         std::begin(response_.rows),
+                         std::end(response_.rows),
+                         [value](auto const& row)
+                         {
+                             auto const row_hash = std::string_view{ reinterpret_cast<char const*>(std::data(row.info_hash)),
+                                                                     std::size(row.info_hash) };
+                             return row_hash == value;
+                         });
+                     it == std::end(response_.rows))
+            {
+                row_.reset();
+            }
+            else
+            {
+                row_ = it - std::begin(response_.rows);
             }
 
             return true;
@@ -589,29 +601,30 @@ void tr_announcerParseHttpScrapeResponse(tr_scrape_response& response, std::stri
 
         bool Int64(int64_t value, Context const& /*context*/) override
         {
-            if (auto const key = currentKey(); row_ && key == "complete"sv)
+            auto const is_value = row_ && depth() == 3U;
+            if (auto const key = currentKey(); is_value && key == "complete"sv)
             {
                 response_.rows[*row_].seeders = value;
             }
-            else if (row_ && key == "downloaded"sv)
+            else if (is_value && key == "downloaded"sv)
             {
                 response_.rows[*row_].downloads = value;
             }
-            else if (row_ && key == "incomplete"sv)
+            else if (is_value && key == "incomplete"sv)
             {
                 response_.rows[*row_].leechers = value;
             }
-            else if (row_ && key == "downloaders"sv)
+            else if (is_value && key == "downloaders"sv)
             {
                 response_.rows[*row_].downloaders = value;
             }
-            else if (key == "min_request_interval"sv)
+            else if (pathIs("flags"sv, "min_request_interval"sv))
             {
                 response_.min_request_interval = static_cast<int>(value);
             }
             else
             {
-                tr_logAddDebug(fmt::format("unexpected key '{}' int '{}'", key, value), log_name_);
+                tr_logAddDebug(fmt::format("unexpected path '{}' int '{}'", path(), value), log_name_);
             }
 
             return true;
@@ -619,13 +632,13 @@ void tr_announcerParseHttpScrapeResponse(tr_scrape_response& response, std::stri
 
         bool String(std::string_view value, Context const& /*context*/) override
         {
-            if (auto const key = currentKey(); depth() == 1 && key == "failure reason"sv)
+            if (pathIs("failure reason"sv))
             {
                 response_.errmsg = value;
             }
             else
             {
-                tr_logAddDebug(fmt::format("unexpected key '{}' str '{}'", key, value), log_name_);
+                tr_logAddDebug(fmt::format("unexpected path '{}' str '{}'", path(), value), log_name_);
             }
 
             return true;

libtransmission/benc.h

@@ -134,6 +134,8 @@ struct BasicHandler : public Handler
         return key(depth());
     }
 
+    static auto constexpr ArrayKey = std::nullopt;
+
 protected:
     [[nodiscard]] std::string path() const
     {
@@ -141,12 +143,26 @@ protected:
         for (size_t i = 0; i <= depth(); ++i)
         {
             ret += '[';
-            ret += key(i);
+            ret += key(i).value_or(std::string_view{});
             ret += ']';
         }
         return ret;
     }
 
+    template<typename... Args>
+    [[nodiscard]] bool pathStartsWith(Args... args) const noexcept
+    {
+        auto i = 1U;
+        return (depth() >= sizeof...(args)) && ((key(i++) == args) && ...);
+    }
+
+    template<typename... Args>
+    [[nodiscard]] bool pathIs(Args... args) const noexcept
+    {
+        auto i = 1U;
+        return (depth() == sizeof...(args)) && ((key(i++) == args) && ...);
+    }
+
 private:
     constexpr void push() noexcept
     {
@@ -160,7 +176,7 @@ private:
     }
 
     size_t depth_ = 0;
-    std::array<std::string_view, MaxDepth> keys_;
+    std::array<std::optional<std::string_view>, MaxDepth> keys_;
 };
 
 template<std::size_t MaxDepth>

libtransmission/torrent-metainfo.cc

@@ -95,11 +95,17 @@ struct MetainfoHandler final : public transmission::benc::BasicHandler<MaxBencDe
     {
         if (state_ == State::FileTree)
         {
+            auto const path_element = currentKey();
+            if (!path_element)
+            {
+                return false;
+            }
+
             if (!std::empty(file_subpath_))
             {
                 file_subpath_ += '/';
             }
-            tr_torrent_files::sanitize_subpath(currentKey(), file_subpath_);
+            tr_torrent_files::sanitize_subpath(*path_element, file_subpath_);
         }
         else if (pathIs(InfoKey))
         {
@@ -165,7 +171,7 @@ struct MetainfoHandler final : public transmission::benc::BasicHandler<MaxBencDe
             file_subpath_.clear();
             file_length_ = 0;
         }
-        else if (pathStartsWith(InfoKey, FilesKey, ""sv, PathUtf8Key))
+        else if (pathStartsWith(InfoKey, FilesKey, ArrayKey, PathUtf8Key))
         {
             // torrent has a utf8 path, drop the other one due to probable non-utf8 encoding
             file_subpath_.clear();
@@ -442,20 +448,6 @@ struct MetainfoHandler final : public transmission::benc::BasicHandler<MaxBencDe
     }
 
 private:
-    template<typename... Args>
-    [[nodiscard]] bool pathStartsWith(Args... args) const noexcept
-    {
-        auto i = 1U;
-        return (depth() >= sizeof...(args)) && ((key(i++) == args) && ...);
-    }
-
-    template<typename... Args>
-    [[nodiscard]] bool pathIs(Args... args) const noexcept
-    {
-        auto i = 1U;
-        return (depth() == sizeof...(args)) && ((key(i++) == args) && ...);
-    }
-
     [[nodiscard]] bool addFile(Context const& context)
     {
         bool ok = true;