From 20aef2f79de1736448f4c8a63e5725eddbb96bd6 Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Mon, 25 Mar 2024 06:09:51 +0800 Subject: [PATCH] fix: misc net.cc and blocklist.cc fixes (#6717) * fix: `tr_address` should be invalid by default * fix: allow loopback address for LPD and incoming connections * fix: `parseCidrline()` should set address type * code review: keep `memcmp` --- libtransmission/announcer-http.cc | 2 +- libtransmission/blocklist.cc | 10 ++++++---- libtransmission/net.cc | 20 +++++++++++++------- libtransmission/net.h | 6 ++++-- libtransmission/peer-io.cc | 5 ----- libtransmission/peer-mgr.cc | 2 +- libtransmission/peer-mgr.h | 9 +++++++-- 7 files changed, 32 insertions(+), 22 deletions(-) diff --git a/libtransmission/announcer-http.cc b/libtransmission/announcer-http.cc index 8c0ce7c40..dafd6053f 100644 --- a/libtransmission/announcer-http.cc +++ b/libtransmission/announcer-http.cc @@ -339,7 +339,7 @@ void tr_announcerParseHttpAnnounceResponse(tr_announce_response& response, std:: { BasicHandler::EndDict(context); - if (pex_.is_valid_for_peers()) + if (pex_.is_valid()) { response_.pex.push_back(pex_); pex_ = {}; diff --git a/libtransmission/blocklist.cc b/libtransmission/blocklist.cc index 336d6f0bc..9ae103efe 100644 --- a/libtransmission/blocklist.cc +++ b/libtransmission/blocklist.cc @@ -204,10 +204,12 @@ std::optional parseCidrLine(std::string_view line) return {}; } - auto const mask = uint32_t{ 0xFFFFFFFF } << (32 - *pflen); - auto const ip_u = htonl(addrpair.first.addr.addr4.s_addr); - addrpair.first.addr.addr4.s_addr = ntohl(ip_u & mask); - addrpair.second.addr.addr4.s_addr = ntohl(ip_u | (~mask)); + auto const mask = ~(~uint32_t{ 0 } >> *pflen); + auto const ip_u = ntohl(addrpair.first.addr.addr4.s_addr); + auto tmp = htonl(ip_u & mask); + std::tie(addrpair.first, std::ignore) = tr_address::from_compact_ipv4(reinterpret_cast(&tmp)); + tmp = htonl(ip_u | (~mask)); + std::tie(addrpair.second, std::ignore) = tr_address::from_compact_ipv4(reinterpret_cast(&tmp)); return addrpair; } diff --git a/libtransmission/net.cc b/libtransmission/net.cc index 295c3dfde..b6d4512af 100644 --- a/libtransmission/net.cc +++ b/libtransmission/net.cc @@ -231,7 +231,7 @@ tr_peer_socket tr_netOpenPeerSocket(tr_session* session, tr_socket_address const TR_ASSERT(addr.is_valid()); TR_ASSERT(!tr_peer_socket::limit_reached(session)); - if (tr_peer_socket::limit_reached(session) || !session->allowsTCP() || !socket_address.is_valid_for_peers()) + if (tr_peer_socket::limit_reached(session) || !session->allowsTCP() || !socket_address.is_valid()) { return {}; } @@ -446,23 +446,29 @@ namespace is_valid_for_peers_helpers /* isMartianAddr was written by Juliusz Chroboczek, and is covered under the same license as third-party/dht/dht.c. */ -[[nodiscard]] auto is_martian_addr(tr_address const& addr) +[[nodiscard]] auto is_martian_addr(tr_address const& addr, tr_peer_from from) { static auto constexpr Zeroes = std::array{}; + auto const loopback_allowed = from == TR_PEER_FROM_INCOMING || from == TR_PEER_FROM_LPD || from == TR_PEER_FROM_RESUME; switch (addr.type) { case TR_AF_INET: { auto const* const address = reinterpret_cast(&addr.addr.addr4); - return address[0] == 0 || address[0] == 127 || (address[0] & 0xE0) == 0xE0; + return address[0] == 0 || // 0.x.x.x + (!loopback_allowed && address[0] == 127) || // 127.x.x.x + (address[0] & 0xE0) == 0xE0; // multicast address } case TR_AF_INET6: { auto const* const address = reinterpret_cast(&addr.addr.addr6); - return address[0] == 0xFF || - (memcmp(address, std::data(Zeroes), 15) == 0 && (address[15] == 0 || address[15] == 1)); + return address[0] == 0xFF || // multicast address + (std::memcmp(address, std::data(Zeroes), 15) == 0 && + (address[15] == 0 || // :: + (!loopback_allowed && address[15] == 1)) // ::1 + ); } default: @@ -703,12 +709,12 @@ std::string tr_socket_address::display_name(tr_address const& address, tr_port p return fmt::format("[{:s}]:{:d}", address.display_name(), port.host()); } -bool tr_socket_address::is_valid_for_peers() const noexcept +bool tr_socket_address::is_valid_for_peers(tr_peer_from from) const noexcept { using namespace is_valid_for_peers_helpers; return is_valid() && !std::empty(port_) && !is_ipv6_link_local_address(address_) && !is_ipv4_mapped_address(address_) && - !is_martian_addr(address_); + !is_martian_addr(address_, from); } std::optional tr_socket_address::from_sockaddr(struct sockaddr const* from) diff --git a/libtransmission/net.h b/libtransmission/net.h index 6e358745a..1499a7868 100644 --- a/libtransmission/net.h +++ b/libtransmission/net.h @@ -60,6 +60,8 @@ using tr_socket_t = int; #define sockerrno errno #endif +#include "libtransmission/transmission.h" // tr_peer_from + #include "libtransmission/tr-assert.h" #include "libtransmission/utils.h" // for tr_compare_3way() @@ -234,7 +236,7 @@ struct tr_address [[nodiscard]] bool is_global_unicast_address() const noexcept; - tr_address_type type; + tr_address_type type = NUM_TR_AF_INET_TYPES; union { struct in6_addr addr6; @@ -306,7 +308,7 @@ struct tr_socket_address return address_.is_valid(); } - [[nodiscard]] bool is_valid_for_peers() const noexcept; + [[nodiscard]] bool is_valid_for_peers(tr_peer_from from) const noexcept; [[nodiscard]] int compare(tr_socket_address const& that) const noexcept { diff --git a/libtransmission/peer-io.cc b/libtransmission/peer-io.cc index 556f81673..4ee09be6d 100644 --- a/libtransmission/peer-io.cc +++ b/libtransmission/peer-io.cc @@ -129,11 +129,6 @@ std::shared_ptr tr_peerIo::new_outgoing( TR_ASSERT(socket_address.is_valid()); TR_ASSERT(utp || session->allowsTCP()); - if (!socket_address.is_valid_for_peers()) - { - return {}; - } - auto peer_io = tr_peerIo::create(session, parent, &info_hash, false, is_seed); auto const func = small::max_size_map, TR_NUM_PREFERRED_TRANSPORT>{ { TR_PREFER_UTP, diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 87c1749af..bec9b9923 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -1439,7 +1439,7 @@ size_t tr_peerMgrAddPex(tr_torrent* tor, tr_peer_from from, tr_pex const* pex, s for (tr_pex const* const end = pex + n_pex; pex != end; ++pex) { if (tr_isPex(pex) && /* safeguard against corrupt data */ - !s->manager->blocklists_.contains(pex->socket_address.address()) && pex->is_valid_for_peers() && + !s->manager->blocklists_.contains(pex->socket_address.address()) && pex->is_valid_for_peers(from) && from != TR_PEER_FROM_INCOMING && (from != TR_PEER_FROM_PEX || (pex->flags & ADDED_F_CONNECTABLE) != 0)) { // we store this peer since it is supposedly connectable (socket address should be the peer's listening address) diff --git a/libtransmission/peer-mgr.h b/libtransmission/peer-mgr.h index 398b78c7c..6835a5e9a 100644 --- a/libtransmission/peer-mgr.h +++ b/libtransmission/peer-mgr.h @@ -517,9 +517,14 @@ struct tr_pex return compare(that) < 0; } - [[nodiscard]] bool is_valid_for_peers() const noexcept + [[nodiscard]] bool is_valid() const noexcept { - return socket_address.is_valid_for_peers(); + return socket_address.is_valid(); + } + + [[nodiscard]] bool is_valid_for_peers(tr_peer_from from) const noexcept + { + return socket_address.is_valid_for_peers(from); } tr_socket_address socket_address;