Skip to content
Browse files

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.
  • Loading branch information...
tomsmeding committed Aug 28, 2019
1 parent 8501481 commit 6bbc646e6f517185344821345459e0dc89ca2c1d
Showing with 10 additions and 3 deletions.
  1. +10 −3 src/cryptonote_core/tx_pool.cpp
@@ -733,7 +733,7 @@ namespace cryptonote
if (meta.double_spend_seen)
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.
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 {
cumulative_num += it->second.txs;
} while (it != agebytes.begin() && cumulative_num < end);
stats.histo_98pc = it->first;
factor = 9;
delta = it->first;

0 comments on commit 6bbc646

Please sign in to comment.
You can’t perform that action at this time.