From 508060e0d8475df725df2e49b9117af3c8531e60 Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Tue, 11 Aug 2026 18:30:56 +0530 Subject: [PATCH 1/2] reject out-of-range port in SplitHostAndPort Signed-off-by: ubeddulla khan --- src/brpc/uri.cpp | 12 +++++++++--- test/brpc_uri_unittest.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/brpc/uri.cpp b/src/brpc/uri.cpp index cb48a43d90..8498a7372b 100644 --- a/src/brpc/uri.cpp +++ b/src/brpc/uri.cpp @@ -86,10 +86,16 @@ inline const char* SplitHostAndPort(const char* host_begin, uint64_t multiply = 1; for (const char* q = host_end - 1; q > host_begin; --q) { if (*q >= '0' && *q <= '9') { - port_raw += (*q - '0') * multiply; - multiply *= 10; + // Stop accumulating once out of range. This avoids uint64 overflow + // of port_raw/multiply and the narrowing to int below, which would + // otherwise turn an out-of-range port into a valid-looking wrong + // one (e.g. ":4294967377" truncating to 81). + if (port_raw <= 65535) { + port_raw += (*q - '0') * multiply; + multiply *= 10; + } } else if (*q == ':') { - *port = static_cast(port_raw); + *port = (port_raw <= 65535) ? static_cast(port_raw) : -1; return q; } else { break; diff --git a/test/brpc_uri_unittest.cpp b/test/brpc_uri_unittest.cpp index 14638b8692..41cda2e607 100644 --- a/test/brpc_uri_unittest.cpp +++ b/test/brpc_uri_unittest.cpp @@ -89,6 +89,37 @@ TEST(URITest, only_host) { ASSERT_EQ(0u, uri.QueryCount()); } +TEST(URITest, out_of_range_port) { + brpc::URI uri; + // 4294967377 == 2^32 + 81. Without a range check the accumulated value + // narrows to int and yields 81, so port() must not return the wrapped port. + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:4294967377/s")); + ASSERT_EQ(-1, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); + ASSERT_EQ("/s", uri.path()); + + // Just above the valid range is rejected too. + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:65536/s")); + ASSERT_EQ(-1, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); + + // A very long run of digits must not overflow the accumulator. + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:999999999999999999999999/s")); + ASSERT_EQ(-1, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); + + // Boundaries of the valid range still parse. + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:65535/s")); + ASSERT_EQ(65535, uri.port()); + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:0/s")); + ASSERT_EQ(0, uri.port()); + + // Host header path goes through the same helper. + uri.SetHostAndPort("www.baidu.com:4294967377"); + ASSERT_EQ(-1, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); +} + TEST(URITest, no_scheme) { brpc::URI uri; ASSERT_EQ(0, uri.SetHttpURL(" user:passwd2@www.baidu1.com/s?wd=uri2&nonkey=22#frag ")); From 3fd0eee81f4204b090c48315b07c207c05521741 Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Sat, 15 Aug 2026 20:06:36 +0530 Subject: [PATCH 2/2] parse port forward and clamp to avoid accumulator wrap --- src/brpc/uri.cpp | 27 ++++++++++++++++----------- test/brpc_uri_unittest.cpp | 13 +++++++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/brpc/uri.cpp b/src/brpc/uri.cpp index 8498a7372b..2881a8e54a 100644 --- a/src/brpc/uri.cpp +++ b/src/brpc/uri.cpp @@ -82,20 +82,25 @@ static void ParseQueries(URI::QueryMap& query_map, const std::string &query) { inline const char* SplitHostAndPort(const char* host_begin, const char* host_end, int* port) { - uint64_t port_raw = 0; - uint64_t multiply = 1; for (const char* q = host_end - 1; q > host_begin; --q) { if (*q >= '0' && *q <= '9') { - // Stop accumulating once out of range. This avoids uint64 overflow - // of port_raw/multiply and the narrowing to int below, which would - // otherwise turn an out-of-range port into a valid-looking wrong - // one (e.g. ":4294967377" truncating to 81). - if (port_raw <= 65535) { - port_raw += (*q - '0') * multiply; - multiply *= 10; - } + continue; } else if (*q == ':') { - *port = (port_raw <= 65535) ? static_cast(port_raw) : -1; + // [q + 1, host_end) is all digits. Accumulate it forward and clamp + // to -1 as soon as it leaves the valid range, so an out-of-range + // port never narrows/wraps into a valid-looking wrong one (e.g. + // ":4294967377" truncating to 81, or a long run of digits + // overflowing the accumulator). This matches the port<0||port>65535 + // rejection in str2endpoint/hostname2endpoint. + int64_t port_raw = 0; + for (const char* p = q + 1; p < host_end; ++p) { + port_raw = port_raw * 10 + (*p - '0'); + if (port_raw > 65535) { + port_raw = -1; + break; + } + } + *port = static_cast(port_raw); return q; } else { break; diff --git a/test/brpc_uri_unittest.cpp b/test/brpc_uri_unittest.cpp index 41cda2e607..b9d6b6508e 100644 --- a/test/brpc_uri_unittest.cpp +++ b/test/brpc_uri_unittest.cpp @@ -108,6 +108,19 @@ TEST(URITest, out_of_range_port) { ASSERT_EQ(-1, uri.port()); ASSERT_EQ("www.baidu.com", uri.host()); + // An out-of-range value padded with a long run of leading zeros must not + // wrap the accumulator into a valid-looking port either. + ASSERT_EQ(0, uri.SetHttpURL( + "foo://www.baidu.com:1000000000000000000000000000000000000" + "0000000000000000000000000000/s")); + ASSERT_EQ(-1, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); + + // Leading zeros on an in-range value still parse to that value. + ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:00080/s")); + ASSERT_EQ(80, uri.port()); + ASSERT_EQ("www.baidu.com", uri.host()); + // Boundaries of the valid range still parse. ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:65535/s")); ASSERT_EQ(65535, uri.port());