From 6bbc646e6f517185344821345459e0dc89ca2c1d Mon Sep 17 00:00:00 2001 From: Tom Smeding Date: Wed, 28 Aug 2019 16:46:31 +0200 Subject: [PATCH] Fix bug in mempool get_transaction_stats histogram calculation The 98th percentile position in the agebytes map was incorrectly calculated: it assumed the transactions in the mempool all have unique timestamps at second-granularity. This commit fixes this by correctly finding the right cumulative number of transactions in the map suffix. This bug could lead to an out-of-bounds write in the rare case that all transactions in the mempool were received (and added to the mempool) at a rate of at least 50 transactions per second. (More specifically, the number of *unique* receive_time values, which have second- granularity, must be at most 2% of the number of transactions in the mempool for this crash to trigger.) If this condition is satisfied, 'it' points to *before* the agebytes map, 'delta' gets a nonsense value, and the value of 'i' in the first stats.histo-filling loop will be out of bounds of stats.histo. --- src/cryptonote_core/tx_pool.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index 49d5a8ccc..5927e116a 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -733,7 +733,7 @@ namespace cryptonote if (meta.double_spend_seen) ++stats.num_double_spends; return true; - }, false, include_unrelayed_txes); + }, false, include_unrelayed_txes); stats.bytes_med = epee::misc_utils::median(weights); if (stats.txs_total > 1) { @@ -746,8 +746,15 @@ namespace cryptonote /* If enough txs, spread the first 98% of results across * the first 9 bins, drop final 2% in last bin. */ - it=agebytes.end(); - for (size_t n=0; n <= end; n++, it--); + it = agebytes.end(); + size_t cumulative_num = 0; + /* Since agebytes is not empty and end is nonzero, the + * below loop can always run at least once. + */ + do { + --it; + cumulative_num += it->second.txs; + } while (it != agebytes.begin() && cumulative_num < end); stats.histo_98pc = it->first; factor = 9; delta = it->first;