fix edge case in socks5 UDP unwrap()

2026-05-06 07:56:47 -04:00 · 2026-05-02 22:26:48 +02:00
parent c5578ee20a
commit 49f73dcb4f
5 changed files with 287 additions and 6 deletions
@@ -937,6 +937,7 @@ TEST_SOURCES = \
  test_tracker.cpp \
  test_truncate.cpp \
  test_transfer.cpp \
+  test_udp_socket.cpp \
  test_upnp.cpp \
  test_url_seed.cpp \
  test_utf8.cpp \
@@ -151,7 +151,6 @@ namespace aux { struct alert_manager; }

 		void wrap(udp::endpoint const& ep, span<char const> p, error_code& ec, udp_send_flags_t flags);
 		void wrap(char const* hostname, int port, span<char const> p, error_code& ec, udp_send_flags_t flags);
-		bool unwrap(udp_socket::packet& pack);

 		udp::socket m_socket;

@@ -169,6 +168,17 @@ namespace aux { struct alert_manager; }

 		bool m_abort:1;
 	};
+
+namespace aux {
+
+	// unwrap a SOCKS5-wrapped UDP datagram in-place. Returns false if the packet
+	// is malformed or truncated and should be ignored. On success, ``pack.data``
+	// is updated to point to the unwrapped payload, and either ``pack.from`` (for
+	// IPv4/IPv6 destinations and resolvable hostnames) or ``pack.hostname`` (for
+	// unresolvable hostnames) is populated.
+	TORRENT_EXTRA_EXPORT bool socks5_unwrap(udp_socket::packet& pack);
+
+}
 }

 #endif
@@ -49,6 +49,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/socks5_stream.hpp" // for socks_error
 #include "libtorrent/aux_/keepalive.hpp"
 #include "libtorrent/aux_/resolver_interface.hpp"
+#include "libtorrent/string_view.hpp"

 #include <cstdlib>
 #include <functional>
@@ -238,7 +239,7 @@ int udp_socket::read(span<packet> pkts, error_code& ec)
 				// if the source IP doesn't match the proxy's, ignore the packet
 				if (p.from != m_socks5_connection->target()) continue;
 				// if we failed to unwrap, silently ignore the packet
-				if (!unwrap(p)) continue;
+				if (!aux::socks5_unwrap(p)) continue;
 			}
 			else
 			{
@@ -401,15 +402,18 @@ void udp_socket::wrap(char const* hostname, int const port, span<char const> p
 	m_socket.send_to(iovec, m_socks5_connection->target(), 0, ec);
 }

+namespace aux {
+
 // unwrap the UDP packet from the SOCKS5 header
 // buf is an in-out parameter. It will be updated
 // return false if the packet should be ignored. It's not a valid Socks5 UDP
 // forwarded packet
-bool udp_socket::unwrap(udp_socket::packet& pack)
+bool socks5_unwrap(udp_socket::packet& pack)
 {
 	using namespace libtorrent::aux;

-	// the minimum socks5 header size
+	// the minimum socks5 header size for an IPv4 forwarded packet:
+	// 2 bytes RSV + 1 byte FRAG + 1 byte ATYP + 4 bytes addr + 2 bytes port
 	auto const size = aux::numeric_cast<int>(pack.data.size());
 	if (size <= 10) return false;

@@ -427,13 +431,16 @@ bool udp_socket::unwrap(udp_socket::packet& pack)
 	}
 	else if (atyp == 4)
 	{
-		// IPv6
+		// IPv6: 4-byte header + 16-byte address + 2-byte port = 22 bytes
+		// minimum. The size <= 10 check above is for IPv4 only.
+		if (size <= 22) return false;
 		pack.from = read_v6_endpoint<udp::endpoint>(p);
 	}
 	else
 	{
 		std::uint8_t const len = read_uint8(p);
-		if (len > pack.data.end() - p) return false;
+		// reserve 2 trailing bytes for the port that follows the hostname
+		if (len + 2 > pack.data.end() - p) return false;
 		string_view hostname(p, len);
 		p += len;

@@ -450,6 +457,8 @@ bool udp_socket::unwrap(udp_socket::packet& pack)
 	return true;
 }

+} // namespace aux
+
 #if !defined BOOST_ASIO_ENABLE_CANCELIO && defined TORRENT_WINDOWS
 #error BOOST_ASIO_ENABLE_CANCELIO needs to be defined when building libtorrent to enable cancel() in asio on windows
 #endif
@@ -181,6 +181,7 @@ run test_receive_buffer.cpp ;
 run test_alert_manager.cpp ;
 run test_apply_pad.cpp ;
 run test_alert_types.cpp ;
+run test_udp_socket.cpp ;
 run test_magnet.cpp ;
 run test_storage.cpp ;
 run test_store_buffer.cpp ;
@@ -312,6 +313,7 @@ alias deterministic-tests :
 	test_torrent
 	test_torrent_info
 	test_torrent_list
+	test_udp_socket
 	test_utf8
 	test_xml
 	test_store_buffer
@@ -0,0 +1,259 @@
+/*
+
+Copyright (c) 2026, Arvid Norberg
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#include "test.hpp"
+#include "libtorrent/udp_socket.hpp"
+#include "libtorrent/socket.hpp"
+#include "libtorrent/span.hpp"
+
+#include <array>
+#include <cstdint>
+#include <cstring>
+#include <vector>
+
+using namespace lt;
+
+namespace {
+
+// build a SOCKS5 UDP forwarded packet for an IPv4 destination.
+// header layout (RFC 1928 section 7):
+//   2 bytes RSV (0x0000)
+//   1 byte  FRAG
+//   1 byte  ATYP (0x01 = IPv4)
+//   4 bytes addr
+//   2 bytes port
+//   N bytes payload
+std::vector<char> make_v4_packet(std::uint8_t const frag
+	, std::array<std::uint8_t, 4> const addr
+	, std::uint16_t const port
+	, span<char const> payload)
+{
+	std::vector<char> buf;
+	buf.push_back(0); buf.push_back(0); // RSV
+	buf.push_back(char(frag));
+	buf.push_back(0x01); // ATYP = IPv4
+	for (auto b : addr) buf.push_back(char(b));
+	buf.push_back(char(port >> 8));
+	buf.push_back(char(port & 0xff));
+	buf.insert(buf.end(), payload.begin(), payload.end());
+	return buf;
+}
+
+// build a SOCKS5 UDP forwarded packet for an IPv6 destination.
+// header layout: 2 RSV + 1 FRAG + 1 ATYP(0x04) + 16 addr + 2 port + payload
+std::vector<char> make_v6_packet(std::uint8_t const frag
+	, std::array<std::uint8_t, 16> const addr
+	, std::uint16_t const port
+	, span<char const> payload)
+{
+	std::vector<char> buf;
+	buf.push_back(0); buf.push_back(0);
+	buf.push_back(char(frag));
+	buf.push_back(0x04);
+	for (auto b : addr) buf.push_back(char(b));
+	buf.push_back(char(port >> 8));
+	buf.push_back(char(port & 0xff));
+	buf.insert(buf.end(), payload.begin(), payload.end());
+	return buf;
+}
+
+// build a SOCKS5 UDP forwarded packet for a domain-name destination.
+// header layout: 2 RSV + 1 FRAG + 1 ATYP(0x03) + 1 LEN + LEN hostname + 2 port + payload
+std::vector<char> make_hostname_packet(std::uint8_t const frag
+	, string_view const hostname
+	, std::uint16_t const port
+	, span<char const> payload)
+{
+	std::vector<char> buf;
+	buf.push_back(0); buf.push_back(0);
+	buf.push_back(char(frag));
+	buf.push_back(0x03);
+	buf.push_back(char(hostname.size()));
+	buf.insert(buf.end(), hostname.begin(), hostname.end());
+	buf.push_back(char(port >> 8));
+	buf.push_back(char(port & 0xff));
+	buf.insert(buf.end(), payload.begin(), payload.end());
+	return buf;
+}
+
+} // anonymous namespace
+
+TORRENT_TEST(socks5_unwrap_ipv4)
+{
+	std::array<char, 4> const payload{{'a', 'b', 'c', 'd'}};
+	auto buf = make_v4_packet(0, {{1, 2, 3, 4}}, 6881, payload);
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(aux::socks5_unwrap(pack));
+	TEST_EQUAL(pack.from.address().to_string(), "1.2.3.4");
+	TEST_EQUAL(pack.from.port(), 6881);
+	TEST_EQUAL(int(pack.data.size()), int(payload.size()));
+	TEST_CHECK(std::memcmp(pack.data.data(), payload.data(), payload.size()) == 0);
+}
+
+TORRENT_TEST(socks5_unwrap_ipv6)
+{
+	std::array<std::uint8_t, 16> const a{{
+		0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01}};
+	std::array<char, 3> const payload{{'x', 'y', 'z'}};
+	auto buf = make_v6_packet(0, a, 1234, payload);
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(aux::socks5_unwrap(pack));
+	TEST_EQUAL(pack.from.address().is_v6(), true);
+	TEST_EQUAL(pack.from.port(), 1234);
+	TEST_EQUAL(int(pack.data.size()), int(payload.size()));
+	TEST_CHECK(std::memcmp(pack.data.data(), payload.data(), payload.size()) == 0);
+}
+
+TORRENT_TEST(socks5_unwrap_hostname_resolvable)
+{
+	// a hostname that parses as a valid address goes into pack.from
+	std::array<char, 2> const payload{{'h', 'i'}};
+	auto buf = make_hostname_packet(0, "5.6.7.8", 9000, payload);
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(aux::socks5_unwrap(pack));
+	TEST_EQUAL(pack.from.address().to_string(), "5.6.7.8");
+	TEST_EQUAL(pack.from.port(), 9000);
+	TEST_EQUAL(int(pack.data.size()), int(payload.size()));
+	TEST_CHECK(std::memcmp(pack.data.data(), payload.data(), payload.size()) == 0);
+}
+
+TORRENT_TEST(socks5_unwrap_hostname_unresolvable)
+{
+	// a hostname that does not parse as an address goes into pack.hostname
+	std::array<char, 2> const payload{{'h', 'i'}};
+	auto buf = make_hostname_packet(0, "example.org", 80, payload);
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(aux::socks5_unwrap(pack));
+	TEST_EQUAL(pack.hostname, "example.org");
+	TEST_EQUAL(int(pack.data.size()), int(payload.size()));
+	TEST_CHECK(std::memcmp(pack.data.data(), payload.data(), payload.size()) == 0);
+}
+
+TORRENT_TEST(socks5_unwrap_reject_fragmented)
+{
+	std::array<char, 1> const payload{{'!'}};
+	auto buf = make_v4_packet(1, {{1, 2, 3, 4}}, 1, payload);
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}
+
+TORRENT_TEST(socks5_unwrap_reject_too_short)
+{
+	// the IPv4 minimum is 10 bytes of header, plus at least one payload byte
+	std::array<char, 10> buf{};
+	buf[3] = 0x01; // ATYP = IPv4
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}
+
+// regression: an IPv6 forwarded packet shorter than the minimum 22-byte
+// header (4 byte preamble + 16 byte address + 2 byte port) was previously
+// only rejected by the IPv4-sized "size <= 10" check, which let unwrap()
+// read past the end of the buffer.
+TORRENT_TEST(socks5_unwrap_reject_truncated_ipv6)
+{
+	// 11 bytes total: passes the size > 10 check, but is well short of the
+	// 22 bytes required to read a v6 endpoint.
+	std::array<char, 11> buf{};
+	buf[3] = 0x04; // ATYP = IPv6
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}
+
+TORRENT_TEST(socks5_unwrap_reject_ipv6_one_short)
+{
+	// exactly 22 bytes is still a header with no payload. The implementation
+	// requires size > 22 to leave room for at least one byte of payload.
+	std::array<char, 22> buf{};
+	buf[3] = 0x04;
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}
+
+// regression: a hostname-ATYP packet whose length byte reaches the end of
+// the buffer leaves no room for the trailing 2-byte port. The previous
+// bounds check only required the hostname to fit, so unwrap() would read
+// 2 bytes past the buffer for the port.
+TORRENT_TEST(socks5_unwrap_reject_hostname_missing_port)
+{
+	// 4 byte preamble + 1 byte LEN + 5 byte hostname = 10 bytes, no port.
+	std::array<char, 10> buf{};
+	buf[3] = 0x03; // ATYP = hostname
+	buf[4] = 5;    // LEN
+	buf[5] = 'h'; buf[6] = 'e'; buf[7] = 'l'; buf[8] = 'l'; buf[9] = 'o';
+	// Must be > 10 bytes to reach the hostname branch, so add one trailing
+	// byte that is too few for a port.
+	std::vector<char> v(buf.begin(), buf.end());
+	v.push_back(0); // only one of the two needed port bytes
+
+	udp_socket::packet pack;
+	pack.data = span<char>{v.data(), int(v.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}
+
+TORRENT_TEST(socks5_unwrap_reject_hostname_overflow)
+{
+	// LEN claims more bytes than are present in the buffer
+	std::array<char, 12> buf{};
+	buf[3] = 0x03;
+	buf[4] = 50; // LEN much larger than what's in the buffer
+
+	udp_socket::packet pack;
+	pack.data = span<char>{buf.data(), int(buf.size())};
+
+	TEST_CHECK(!aux::socks5_unwrap(pack));
+}