diff options
author | Tom Smeding <tom.smeding@gmail.com> | 2019-08-28 16:46:31 +0200 |
---|---|---|
committer | Tom Smeding <tom.smeding@gmail.com> | 2019-08-28 16:46:31 +0200 |
commit | 6bbc646e6f517185344821345459e0dc89ca2c1d (patch) | |
tree | c88a1dcf0d4e132e14464d5e352d58beb3c330e7 /src | |
parent | Merge pull request #5707 (diff) | |
download | monero-6bbc646e6f517185344821345459e0dc89ca2c1d.tar.xz |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/cryptonote_core/tx_pool.cpp | 13 |
1 files 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; |