Skip to content

Commit

Permalink
Validate port value in SplitHostPort
Browse files Browse the repository at this point in the history 
Forward the validation of the port from `ParseUInt16(...)`.
Consider port 0 as invalid.
Add suitable test for the `SplitHostPort` function.
Add doxygen description to the `SplitHostPort` function.

Github-Pull: bitcoin#22087
Rebased-From: b7a6d95
  • Loading branch information
@amadeuszpawlik @luke-jr
amadeuszpawlik authored and luke-jr committed Apr 13, 2022
1 parent 9fce2b4 commit 0a9c31f
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 24 deletions.
53 changes: 35 additions & 18 deletions src/test/netbase_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,31 +83,48 @@ BOOST_AUTO_TEST_CASE(netbase_properties)

}

bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port)
bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port, bool validPort)
{
std::string hostOut;
uint16_t portOut{0};
SplitHostPort(test, portOut, hostOut);
return hostOut == host && port == portOut;
bool validPortOut = SplitHostPort(test, portOut, hostOut);
return hostOut == host && portOut == port && validPortOut == validPort;
}

BOOST_AUTO_TEST_CASE(netbase_splithost)
{
BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", 0));
BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", 0));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:80", "www.bitcoincore.org", 80));
BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]:80", "www.bitcoincore.org", 80));
BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", 0));
BOOST_CHECK(TestSplitHost("127.0.0.1:8333", "127.0.0.1", 8333));
BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", 0));
BOOST_CHECK(TestSplitHost("[127.0.0.1]:8333", "127.0.0.1", 8333));
BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", 0));
BOOST_CHECK(TestSplitHost("[::ffff:127.0.0.1]:8333", "::ffff:127.0.0.1", 8333));
BOOST_CHECK(TestSplitHost("[::]:8333", "::", 8333));
BOOST_CHECK(TestSplitHost("::8333", "::8333", 0));
BOOST_CHECK(TestSplitHost(":8333", "", 8333));
BOOST_CHECK(TestSplitHost("[]:8333", "", 8333));
BOOST_CHECK(TestSplitHost("", "", 0));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", 0, true));
BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", 0, true));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:80", "www.bitcoincore.org", 80, true));
BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]:80", "www.bitcoincore.org", 80, true));
BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", 0, true));
BOOST_CHECK(TestSplitHost("127.0.0.1:8333", "127.0.0.1", 8333, true));
BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", 0, true));
BOOST_CHECK(TestSplitHost("[127.0.0.1]:8333", "127.0.0.1", 8333, true));
BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", 0, true));
BOOST_CHECK(TestSplitHost("[::ffff:127.0.0.1]:8333", "::ffff:127.0.0.1", 8333, true));
BOOST_CHECK(TestSplitHost("[::]:8333", "::", 8333, true));
BOOST_CHECK(TestSplitHost("::8333", "::8333", 0, true));
BOOST_CHECK(TestSplitHost(":8333", "", 8333, true));
BOOST_CHECK(TestSplitHost("[]:8333", "", 8333, true));
BOOST_CHECK(TestSplitHost("", "", 0, true));
BOOST_CHECK(TestSplitHost(":65535", "", 65535, true));
BOOST_CHECK(TestSplitHost(":65536", "", 0, false));
BOOST_CHECK(TestSplitHost(":-1", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:70001", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:-1", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:-0", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:0", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:1/2", "", 0, false));
BOOST_CHECK(TestSplitHost("[]:1E2", "", 0, false));
BOOST_CHECK(TestSplitHost("127.0.0.1:65536", "127.0.0.1", 0, false));
BOOST_CHECK(TestSplitHost("127.0.0.1:0", "127.0.0.1", 0, false));
BOOST_CHECK(TestSplitHost("127.0.0.1:", "127.0.0.1", 0, false));
BOOST_CHECK(TestSplitHost("127.0.0.1:1/2", "127.0.0.1", 0, false));
BOOST_CHECK(TestSplitHost("127.0.0.1:1E2", "127.0.0.1", 0, false));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:65536", "www.bitcoincore.org", 0, false));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:0", "www.bitcoincore.org", 0, false));
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:", "www.bitcoincore.org", 0, false));
}

bool static TestParse(std::string src, std::string canon)
Expand Down
18 changes: 13 additions & 5 deletions src/util/strencodings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,25 +107,33 @@ std::vector<unsigned char> ParseHex(const std::string& str)
return ParseHex(str.c_str());
}

void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut)
bool SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut)
{
bool valid = false;
size_t colon = in.find_last_of(':');
// if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator
bool fHaveColon = colon != in.npos;
bool fBracketed = fHaveColon && (in[0] == '[' && in[colon - 1] == ']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe
bool fMultiColon{fHaveColon && colon != 0 && (in.find_last_of(':', colon - 1) != in.npos)};
if (fHaveColon && (colon == 0 || fBracketed || !fMultiColon)) {
uint16_t n;
if (ParseUInt16(in.substr(colon + 1), &n)) {
in = in.substr(0, colon);
portOut = n;
if (!in.substr(colon + 1).empty()) {
uint16_t n;
if (ParseUInt16(in.substr(colon+1), &n)) {
portOut = n;
valid = (portOut != 0);
}
}
in = in.substr(0, colon);
} else {
valid = true;
}
if (in.size() > 0 && in[0] == '[' && in[in.size() - 1] == ']') {
hostOut = in.substr(1, in.size() - 2);
} else {
hostOut = in;
}

return valid;
}

std::string EncodeBase64(Span<const unsigned char> input)
Expand Down
11 changes: 10 additions & 1 deletion src/util/strencodings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,16 @@ std::string EncodeBase32(Span<const unsigned char> input, bool pad = true);
*/
std::string EncodeBase32(const std::string& str, bool pad = true);

void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut);
/**
* Splits socket address string into host string and port value.
* Validates port value.
*
* @param[in] in The socket address string to split.
* @param[out] portOut Port-portion of the input, if found and parsable.
* @param[out] hostOut Host-portion of the input, if found.
* @return true if port-portion is absent or within its allowed range, otherwise false
*/
bool SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut);

// LocaleIndependentAtoi is provided for backwards compatibility reasons.
//
Expand Down

0 comments on commit 0a9c31f

Please sign in to comment.