Skip to content

Commit 859da12

Browse files
kwvgkwvg
committed
merge bitcoin#22087: Validate port-options
1 parent 272c951 commit 859da12

File tree

8 files changed

+131
-14
lines changed

8 files changed

+131
-14
lines changed

‎doc/release-notes-6634.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Updated settings
2+
----------------
3+
4+
- Ports specified in `-port` and `-rpcport` options are now validated at startup. Values that previously worked and were considered valid can now result in errors.

‎src/init.cpp‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,6 +1674,65 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16741674
}
16751675
}
16761676

1677+
// Check port numbers
1678+
for (const std::string port_option : {
1679+
"-port",
1680+
"-rpcport",
1681+
}) {
1682+
if (args.IsArgSet(port_option)) {
1683+
const std::string port = args.GetArg(port_option, "");
1684+
uint16_t n;
1685+
if (!ParseUInt16(port, &n) || n == 0) {
1686+
return InitError(InvalidPortErrMsg(port_option, port));
1687+
}
1688+
}
1689+
}
1690+
1691+
for (const std::string port_option : {
1692+
"-i2psam",
1693+
"-onion",
1694+
"-proxy",
1695+
"-rpcbind",
1696+
"-torcontrol",
1697+
"-whitebind",
1698+
"-zmqpubhashblock",
1699+
"-zmqpubhashchainlock",
1700+
"-zmqpubhashgovernanceobject",
1701+
"-zmqpubhashgovernancevote",
1702+
"-zmqpubhashinstantsenddoublespend",
1703+
"-zmqpubhashrecoveredsig",
1704+
"-zmqpubhashtx",
1705+
"-zmqpubhashtxlock",
1706+
"-zmqpubrawblock",
1707+
"-zmqpubrawchainlock",
1708+
"-zmqpubrawchainlocksig",
1709+
"-zmqpubrawgovernancevote",
1710+
"-zmqpubrawgovernanceobject",
1711+
"-zmqpubrawinstantsenddoublespend",
1712+
"-zmqpubrawrecoveredsig",
1713+
"-zmqpubrawtx",
1714+
"-zmqpubrawtxlock",
1715+
"-zmqpubrawtxlocksig",
1716+
"-zmqpubsequence",
1717+
}) {
1718+
for (const std::string& socket_addr : args.GetArgs(port_option)) {
1719+
std::string host_out;
1720+
uint16_t port_out{0};
1721+
if (!SplitHostPort(socket_addr, port_out, host_out)) {
1722+
return InitError(InvalidPortErrMsg(port_option, socket_addr));
1723+
}
1724+
}
1725+
}
1726+
1727+
for (const std::string& socket_addr : args.GetArgs("-bind")) {
1728+
std::string host_out;
1729+
uint16_t port_out{0};
1730+
std::string bind_socket_addr = socket_addr.substr(0, socket_addr.rfind('='));
1731+
if (!SplitHostPort(bind_socket_addr, port_out, host_out)) {
1732+
return InitError(InvalidPortErrMsg("-bind", socket_addr));
1733+
}
1734+
}
1735+
16771736
// sanitize comments per BIP-0014, format user agent and check total size
16781737
std::vector<std::string> uacomments;
16791738

‎src/test/netbase_tests.cpp‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ BOOST_AUTO_TEST_CASE(netbase_properties)
8383

8484
}
8585

86-
bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port)
86+
bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port, bool validPort=true)
8787
{
8888
std::string hostOut;
8989
uint16_t portOut{0};
90-
SplitHostPort(test, portOut, hostOut);
91-
return hostOut == host && port == portOut;
90+
bool validPortOut = SplitHostPort(test, portOut, hostOut);
91+
return hostOut == host && portOut == port && validPortOut == validPort;
9292
}
9393

9494
BOOST_AUTO_TEST_CASE(netbase_splithost)
@@ -108,6 +108,23 @@ BOOST_AUTO_TEST_CASE(netbase_splithost)
108108
BOOST_CHECK(TestSplitHost(":9999", "", 9999));
109109
BOOST_CHECK(TestSplitHost("[]:9999", "", 9999));
110110
BOOST_CHECK(TestSplitHost("", "", 0));
111+
BOOST_CHECK(TestSplitHost(":65535", "", 65535));
112+
BOOST_CHECK(TestSplitHost(":65536", ":65536", 0, false));
113+
BOOST_CHECK(TestSplitHost(":-1", ":-1", 0, false));
114+
BOOST_CHECK(TestSplitHost("[]:70001", "[]:70001", 0, false));
115+
BOOST_CHECK(TestSplitHost("[]:-1", "[]:-1", 0, false));
116+
BOOST_CHECK(TestSplitHost("[]:-0", "[]:-0", 0, false));
117+
BOOST_CHECK(TestSplitHost("[]:0", "", 0, false));
118+
BOOST_CHECK(TestSplitHost("[]:1/2", "[]:1/2", 0, false));
119+
BOOST_CHECK(TestSplitHost("[]:1E2", "[]:1E2", 0, false));
120+
BOOST_CHECK(TestSplitHost("127.0.0.1:65536", "127.0.0.1:65536", 0, false));
121+
BOOST_CHECK(TestSplitHost("127.0.0.1:0", "127.0.0.1", 0, false));
122+
BOOST_CHECK(TestSplitHost("127.0.0.1:", "127.0.0.1:", 0, false));
123+
BOOST_CHECK(TestSplitHost("127.0.0.1:1/2", "127.0.0.1:1/2", 0, false));
124+
BOOST_CHECK(TestSplitHost("127.0.0.1:1E2", "127.0.0.1:1E2", 0, false));
125+
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:65536", "www.bitcoincore.org:65536", 0, false));
126+
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:0", "www.bitcoincore.org", 0, false));
127+
BOOST_CHECK(TestSplitHost("www.bitcoincore.org:", "www.bitcoincore.org:", 0, false));
111128
}
112129

113130
bool static TestParse(std::string src, std::string canon)

‎src/util/error.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBi
4141
return strprintf(_("Cannot resolve -%s address: '%s'"), optname, strBind);
4242
}
4343

44+
bilingual_str InvalidPortErrMsg(const std::string& optname, const std::string& invalid_value)
45+
{
46+
return strprintf(_("Invalid port specified in %s: '%s'"), optname, invalid_value);
47+
}
48+
4449
bilingual_str AmountHighWarn(const std::string& optname)
4550
{
4651
return strprintf(_("%s is set very high!"), optname);

‎src/util/error.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ bilingual_str TransactionErrorString(const TransactionError error);
3636

3737
bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind);
3838

39+
bilingual_str InvalidPortErrMsg(const std::string& optname, const std::string& strPort);
40+
3941
bilingual_str AmountHighWarn(const std::string& optname);
4042

4143
bilingual_str AmountErrMsg(const std::string& optname, const std::string& strValue);

‎src/util/strencodings.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ std::vector<Byte> ParseHex(std::string_view str)
9797
template std::vector<std::byte> ParseHex(std::string_view);
9898
template std::vector<uint8_t> ParseHex(std::string_view);
9999

100-
void SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut)
100+
bool SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut)
101101
{
102+
bool valid = false;
102103
size_t colon = in.find_last_of(':');
103104
// if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator
104105
bool fHaveColon = colon != in.npos;
@@ -109,13 +110,18 @@ void SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut)
109110
if (ParseUInt16(in.substr(colon + 1), &n)) {
110111
in = in.substr(0, colon);
111112
portOut = n;
113+
valid = (portOut != 0);
112114
}
115+
} else {
116+
valid = true;
113117
}
114118
if (in.size() > 0 && in[0] == '[' && in[in.size() - 1] == ']') {
115119
hostOut = in.substr(1, in.size() - 2);
116120
} else {
117121
hostOut = in;
118122
}
123+
124+
return valid;
119125
}
120126

121127
std::string EncodeBase64(Span<const unsigned char> input)

‎src/util/strencodings.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,16 @@ std::string EncodeBase32(Span<const unsigned char> input, bool pad = true);
8585
*/
8686
std::string EncodeBase32(std::string_view str, bool pad = true);
8787

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

9099
// LocaleIndependentAtoi is provided for backwards compatibility reasons.
91100
//

‎test/functional/feature_proxy.py‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -318,19 +318,34 @@ def networks_dict(d):
318318

319319
self.stop_node(1)
320320

321-
self.log.info("Test passing invalid -proxy raises expected init error")
322-
self.nodes[1].extra_args = ["-proxy=abc:def"]
323-
msg = "Error: Invalid -proxy address or hostname: 'abc:def'"
321+
self.log.info("Test passing invalid -proxy hostname raises expected init error")
322+
self.nodes[1].extra_args = ["-proxy=abc..abc:23456"]
323+
msg = "Error: Invalid -proxy address or hostname: 'abc..abc:23456'"
324324
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
325325

326-
self.log.info("Test passing invalid -onion raises expected init error")
327-
self.nodes[1].extra_args = ["-onion=xyz:abc"]
328-
msg = "Error: Invalid -onion address or hostname: 'xyz:abc'"
326+
self.log.info("Test passing invalid -proxy port raises expected init error")
327+
self.nodes[1].extra_args = ["-proxy=192.0.0.1:def"]
328+
msg = "Error: Invalid port specified in -proxy: '192.0.0.1:def'"
329329
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
330330

331-
self.log.info("Test passing invalid -i2psam raises expected init error")
332-
self.nodes[1].extra_args = ["-i2psam=def:xyz"]
333-
msg = "Error: Invalid -i2psam address or hostname: 'def:xyz'"
331+
self.log.info("Test passing invalid -onion hostname raises expected init error")
332+
self.nodes[1].extra_args = ["-onion=xyz..xyz:23456"]
333+
msg = "Error: Invalid -onion address or hostname: 'xyz..xyz:23456'"
334+
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
335+
336+
self.log.info("Test passing invalid -onion port raises expected init error")
337+
self.nodes[1].extra_args = ["-onion=192.0.0.1:def"]
338+
msg = "Error: Invalid port specified in -onion: '192.0.0.1:def'"
339+
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
340+
341+
self.log.info("Test passing invalid -i2psam hostname raises expected init error")
342+
self.nodes[1].extra_args = ["-i2psam=def..def:23456"]
343+
msg = "Error: Invalid -i2psam address or hostname: 'def..def:23456'"
344+
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
345+
346+
self.log.info("Test passing invalid -i2psam port raises expected init error")
347+
self.nodes[1].extra_args = ["-i2psam=192.0.0.1:def"]
348+
msg = "Error: Invalid port specified in -i2psam: '192.0.0.1:def'"
334349
self.nodes[1].assert_start_raises_init_error(expected_msg=msg)
335350

336351
self.log.info("Test passing invalid -onlynet=i2p without -i2psam raises expected init error")

0 commit comments

Comments
 (0)