aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorWladimir J. van der Laan <laanwj@gmail.com>2018-11-23 09:49:41 +0100
committerWladimir J. van der Laan <laanwj@gmail.com>2018-11-23 09:53:20 +0100
commit2479b779aae435d45062cea902dbb7638e14b88f (patch)
tree3d97273c78dbd2a49206daa37638ff8fb1a2b333 /src
parenta0d86815cda982d06db333fd9290aa82660807cb (diff)
parentb7b36decaf878a8c1dcfdb4a27196c730043474b (diff)
Merge #14728: fix uninitialized read when stringifying an addrLocal
b7b36decaf878a8c1dcfdb4a27196c730043474b fix uninitialized read when stringifying an addrLocal (Kaz Wesley) 8ebbef016928811756e46b9086067d1c826797a8 add test demonstrating addrLocal UB (Kaz Wesley) Pull request description: Reachable from either place where SetIP is used when all of: - our best-guess addrLocal for a peer is IPv4 - the peer tells us it's reaching us at an IPv6 address - NET logging is enabled In that case, SetIP turns an IPv4 address into an IPv6 address without setting the scopeId, which is subsequently read in GetSockAddr during CNetAddr::ToStringIP and passed to getnameinfo. Fix by ensuring every constructor initializes the scopeId field with something. Tree-SHA512: 8f0159750995e08b985335ccf60a273ebd09003990bcf2c3838b550ed8dc2659552ac7611650e6dd8e29d786fe52ed57674f5880f2e18dc594a7a863134739e3
Diffstat (limited to 'src')
-rw-r--r--src/netaddress.cpp1
-rw-r--r--src/netaddress.h2
-rw-r--r--src/test/net_tests.cpp38
3 files changed, 39 insertions, 2 deletions
diff --git a/src/netaddress.cpp b/src/netaddress.cpp
index 72be77dfd9..a0c7f8e3c2 100644
--- a/src/netaddress.cpp
+++ b/src/netaddress.cpp
@@ -17,7 +17,6 @@ static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87,
CNetAddr::CNetAddr()
{
memset(ip, 0, sizeof(ip));
- scopeId = 0;
}
void CNetAddr::SetIP(const CNetAddr& ipIn)
diff --git a/src/netaddress.h b/src/netaddress.h
index 86c13b3465..ca435d17dc 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -33,7 +33,7 @@ class CNetAddr
{
protected:
unsigned char ip[16]; // in network byte order
- uint32_t scopeId; // for scoped/link-local ipv6 addresses
+ uint32_t scopeId{0}; // for scoped/link-local ipv6 addresses
public:
CNetAddr();
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 213afed730..c1ee231d8a 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -189,4 +189,42 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK(pnode2->fFeeler == false);
}
+// prior to PR #14728, this test triggers an undefined behavior
+BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
+{
+ // set up local addresses; all that's necessary to reproduce the bug is
+ // that a normal IPv4 address is among the entries, but if this address is
+ // !IsRoutable the undefined behavior is easier to trigger deterministically
+ {
+ LOCK(cs_mapLocalHost);
+ in_addr ipv4AddrLocal;
+ ipv4AddrLocal.s_addr = 0x0100007f;
+ CNetAddr addr = CNetAddr(ipv4AddrLocal);
+ LocalServiceInfo lsi;
+ lsi.nScore = 23;
+ lsi.nPort = 42;
+ mapLocalHost[addr] = lsi;
+ }
+
+ // create a peer with an IPv4 address
+ in_addr ipv4AddrPeer;
+ ipv4AddrPeer.s_addr = 0xa0b0c001;
+ CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
+ std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, false);
+ pnode->fSuccessfullyConnected.store(true);
+
+ // the peer claims to be reaching us via IPv6
+ in6_addr ipv6AddrLocal;
+ memset(ipv6AddrLocal.s6_addr, 0, 16);
+ ipv6AddrLocal.s6_addr[0] = 0xcc;
+ CAddress addrLocal = CAddress(CService(ipv6AddrLocal, 7777), NODE_NETWORK);
+ pnode->SetAddrLocal(addrLocal);
+
+ // before patch, this causes undefined behavior detectable with clang's -fsanitize=memory
+ AdvertiseLocal(&*pnode);
+
+ // suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer
+ BOOST_CHECK(1);
+}
+
BOOST_AUTO_TEST_SUITE_END()