Skip to content

Commit 2479b77

Browse files
committed
Merge #14728: fix uninitialized read when stringifying an addrLocal
b7b36de fix uninitialized read when stringifying an addrLocal (Kaz Wesley) 8ebbef0 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
2 parents a0d8681 + b7b36de commit 2479b77

File tree

3 files changed

+39
-2
lines changed

3 files changed

+39
-2
lines changed

src/netaddress.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87,
1717
CNetAddr::CNetAddr()
1818
{
1919
memset(ip, 0, sizeof(ip));
20-
scopeId = 0;
2120
}
2221

2322
void CNetAddr::SetIP(const CNetAddr& ipIn)

src/netaddress.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class CNetAddr
3333
{
3434
protected:
3535
unsigned char ip[16]; // in network byte order
36-
uint32_t scopeId; // for scoped/link-local ipv6 addresses
36+
uint32_t scopeId{0}; // for scoped/link-local ipv6 addresses
3737

3838
public:
3939
CNetAddr();

src/test/net_tests.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,42 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
189189
BOOST_CHECK(pnode2->fFeeler == false);
190190
}
191191

192+
// prior to PR #14728, this test triggers an undefined behavior
193+
BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
194+
{
195+
// set up local addresses; all that's necessary to reproduce the bug is
196+
// that a normal IPv4 address is among the entries, but if this address is
197+
// !IsRoutable the undefined behavior is easier to trigger deterministically
198+
{
199+
LOCK(cs_mapLocalHost);
200+
in_addr ipv4AddrLocal;
201+
ipv4AddrLocal.s_addr = 0x0100007f;
202+
CNetAddr addr = CNetAddr(ipv4AddrLocal);
203+
LocalServiceInfo lsi;
204+
lsi.nScore = 23;
205+
lsi.nPort = 42;
206+
mapLocalHost[addr] = lsi;
207+
}
208+
209+
// create a peer with an IPv4 address
210+
in_addr ipv4AddrPeer;
211+
ipv4AddrPeer.s_addr = 0xa0b0c001;
212+
CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
213+
std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, false);
214+
pnode->fSuccessfullyConnected.store(true);
215+
216+
// the peer claims to be reaching us via IPv6
217+
in6_addr ipv6AddrLocal;
218+
memset(ipv6AddrLocal.s6_addr, 0, 16);
219+
ipv6AddrLocal.s6_addr[0] = 0xcc;
220+
CAddress addrLocal = CAddress(CService(ipv6AddrLocal, 7777), NODE_NETWORK);
221+
pnode->SetAddrLocal(addrLocal);
222+
223+
// before patch, this causes undefined behavior detectable with clang's -fsanitize=memory
224+
AdvertiseLocal(&*pnode);
225+
226+
// suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer
227+
BOOST_CHECK(1);
228+
}
229+
192230
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)