aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Smeding <tom.smeding@gmail.com>2019-08-28 16:46:31 +0200
committerTom Smeding <tom.smeding@gmail.com>2019-08-28 16:46:31 +0200
commit6bbc646e6f517185344821345459e0dc89ca2c1d (patch)
treec88a1dcf0d4e132e14464d5e352d58beb3c330e7
parentMerge pull request #5707 (diff)
downloadmonero-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.
-rw-r--r--src/cryptonote_core/tx_pool.cpp13
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;