Monero · Report #3547349
Inverted ternary in peerlist_manager::filter() allows unlimited whitelist entries per host via different ports
How it was found
Summary:
peerlist_manager::filter() in src/p2p/net_peerlist.h has incorrect ternary that operates on the wrong peer list.
When called with white=true it filters the gray list instead of the white list.
As a result it allows a single host to accumulate unlimited white list entries on different ports.
Let's look at the code https://github.com/monero-project/monero/blob/master/src/p2p/net_peerlist.h#L531
template<typename F> size_t peerlist_manager::filter(bool white, const F &f)
{
size_t filtered = 0;
TRY_ENTRY();
CRITICAL_REGION_LOCAL(m_peerlist_lock);
peers_indexed::index<by_addr>::type& sorted_index = white ? m_peers_gray.get<by_addr>() : m_peers_white.get<by_addr>();
auto i = sorted_index.begin();
while (i != sorted_index.end())
{
if (f(*i))
{
i = sorted_index.erase(i);
++filtered;
}
else
++i;
}
CATCH_ENTRY_L0("peerlist_manager::filter()", filtered);
return filtered;
}
Here peers_indexed::index<by_addr>::type& sorted_index = white ? m_peers_gray.get<by_addr>() : m_peers_white.get<by_addr>(); you can see that function operates on a gray list instead of white list.
Compare it with another function, which is correct:
//--------------------------------------------------------------------------------------------------
template<typename F> inline
bool peerlist_manager::foreach(bool white, const F &f)
{
CRITICAL_REGION_LOCAL(m_peerlist_lock);
peers_indexed::index<by_time>::type& by_time_index = white ? m_peers_white.get<by_time>() : m_peers_gray.get<by_time>();
for(const peers_indexed::value_type& vl: boost::adaptors::reverse(by_time_index))
if (!f(vl))
return false;
return true;
}
Releases Affected:
- latest master branch
$ git rev-parse master
f99ee72d4b5065ddcaba2b74fcc6c8dc9be385dc
Steps To Reproduce:
- add test to tests/unit_tests/test_peerlist.cpp
// XXXKE peerlist_manager::filter() has inverted ternary
TEST(peer_list, filter_inverted_ternary_bug)
{
nodetool::peerlist_manager plm;
plm.init(nodetool::peerlist_types{}, false);
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(93,184,216,34, 18080);
ple.id = 9999;
ple.last_seen = 1000;
plm.append_with_peer_gray(ple);
}
ASSERT_EQ(plm.get_gray_peers_count(), 1u);
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(45,33,32,1, 28080);
ple.id = 8888;
ple.last_seen = 1000;
plm.append_with_peer_gray(ple);
}
ASSERT_EQ(plm.get_gray_peers_count(), 2u);
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(45,33,32,1, 18080);
ple.id = 1001;
ple.last_seen = 2000;
plm.append_with_peer_white(ple);
}
std::cout << "[Step 1] After adding 45.33.32.1:18080 to white:" << std::endl;
std::cout << " white_count = " << plm.get_white_peers_count()
<< " (expect 1)" << std::endl;
std::cout << " gray_count = " << plm.get_gray_peers_count()
<< std::endl;
EXPECT_EQ(plm.get_white_peers_count(), 1u);
EXPECT_EQ(plm.get_gray_peers_count(), 2u)
<< "filter(true,...) evicted from gray list instead of white list. "
"Gray entry 45.33.32.1:28080 was wrongly removed.";
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(45,33,32,1, 18081);
ple.id = 1002;
ple.last_seen = 3000;
plm.append_with_peer_white(ple);
}
std::cout << "[Step 2] After adding 45.33.32.1:18081 to white:" << std::endl;
std::cout << " white_count = " << plm.get_white_peers_count()
<< " (expect 1 if fixed, 2 if buggy)" << std::endl;
std::cout << " gray_count = " << plm.get_gray_peers_count() << std::endl;
EXPECT_EQ(plm.get_white_peers_count(), 1u)
<< "Same host (45.33.32.1) has multiple white list entries "
"on different ports. evict_host_from_peerlist did not clean white list. ";
nodetool::peerlist_manager plm2;
plm2.init(nodetool::peerlist_types{}, false);
for (int i = 1; i <= 3; i++)
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(104,16,0,i, 18080);
ple.id = 2000 + i;
ple.last_seen = 1000 + i;
plm2.append_with_peer_white(ple);
}
ASSERT_EQ(plm2.get_white_peers_count(), 3u);
for (int port = 0; port < 20; port++)
{
nodetool::peerlist_entry ple{};
ple.adr = MAKE_IPV4_ADDRESS(45,33,32,1, 18080 + port);
ple.id = 5000 + port;
ple.last_seen = 5000 + port;
plm2.append_with_peer_white(ple);
}
std::cout << "After 3 honest peers + 20 attacker ports on 45.33.32.1:" << std::endl;
std::cout << " white_count = " << plm2.get_white_peers_count()
<< "" << std::endl;
std::set<std::string> unique_hosts;
size_t attacker_entries = 0;
plm2.foreach(true, [&](const nodetool::peerlist_entry &pe) {
unique_hosts.insert(pe.adr.host_str());
if (pe.adr.host_str() == "45.33.32.1")
attacker_entries++;
return true;
});
std::cout << " unique_hosts = " << unique_hosts.size() << std::endl;
std::cout << " attacker_entries = " << attacker_entries
<< "" << std::endl;
}
- save this file as Dockerfile.test-peerlist
FROM ubuntu:22.04 AS builder
RUN set -ex && \
apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get --no-install-recommends --yes install \
build-essential \
ca-certificates \
cmake \
git \
pkg-config \
libboost-all-dev \
libssl-dev \
libzmq3-dev \
libunbound-dev \
libsodium-dev \
libreadline-dev \
libexpat1-dev \
libunwind-dev \
liblzma-dev \
libhidapi-dev \
libusb-1.0-0-dev \
libprotobuf-dev \
protobuf-compiler \
libgtest-dev
WORKDIR /src
COPY . .
RUN set -ex && \
git submodule init && git submodule update && \
mkdir -p build/release && cd build/release && \
cmake -D CMAKE_BUILD_TYPE=Release -D BUILD_TESTS=ON ../.. && \
make -j$(nproc) unit_tests
# Run only the peerlist filter bug test
CMD ["./build/release/tests/unit_tests/unit_tests", "--gtest_filter=peer_list.filter_inverted_ternary_bug"]
- build and run docker image (inside a monero source code)
$ docker build -f Dockerfile.test-peerlist -t monero-peerlist-test .
$ docker run --rm monero-peerlist-test
Note: Google Test filter = peer_list.filter_inverted_ternary_bug
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from peer_list
[ RUN ] peer_list.filter_inverted_ternary_bug
[Step 1] After adding 45.33.32.1:18080 to white:
white_count = 1 (expect 1)
gray_count = 1
/src/tests/unit_tests/test_peerlist.cpp:113: Failure
Expected equality of these values:
plm.get_gray_peers_count()
Which is: 1
2u
Which is: 2
filter(true,...) evicted from gray list instead of white list. Gray entry 45.33.32.1:28080 was wrongly removed.
[Step 2] After adding 45.33.32.1:18081 to white:
white_count = 2 (expect 1 if fixed, 2 if buggy)
gray_count = 1
/src/tests/unit_tests/test_peerlist.cpp:129: Failure
Expected equality of these values:
plm.get_white_peers_count()
Which is: 2
1u
Which is: 1
Same host (45.33.32.1) has multiple white list entries on different ports. evict_host_from_peerlist did not clean white list.
After 3 honest peers + 20 attacker ports on 45.33.32.1:
white_count = 23
unique_hosts = 4
attacker_entries = 20
[ FAILED ] peer_list.filter_inverted_ternary_bug (0 ms)
[----------] 1 test from peer_list (0 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] peer_list.filter_inverted_ternary_bug
Supporting Material/References:
None
Housekeeping
- Be sure to read our policy before submitting
- Provide an XMR address within the report if you wish to receive bounty (assuming that the report is valid)
- PoC within a report will most likely result in more bounty than not
My XMR address is 46NmUkU8JBRNCZEU8MP4YGap4BkepR1dYYn1Cgn6W47KJ6Hy5Kq1vkA7VRdJMEPL9gDUViEFtS14AZuRiiVomYP4Dz9mmSb