wallet2: fix infinite loop in estimate_tx_size_and_weight on large n_outputs
What the developer wrote
Authored by Masamune
wallet2: fix infinite loop in estimate_tx_size_and_weight on large n_outputs
estimate_rct_tx_size() and estimate_tx_weight() compute the Bulletproof padded
output count with `while ((1<<log_padded_outputs) < n_outputs)`, shifting a
signed int. For n_outputs > 2^30 this reaches `1 << 31` (signed overflow, UB),
and once the shift count exceeds the int width the value cycles and never
reaches n_outputs, so the loop never terminates. estimate_tx_size_and_weight()
accepts n_outputs up to INT_MAX (only negative is rejected), so a single
wallet-rpc call spins the handling thread forever.
Shift an unsigned 64-bit one and compare in uint64_t at all three shift sites,
and accumulate the estimated size in size_t (cast the per-input and per-output
terms) so the size arithmetic cannot overflow for large counts.
What changed, and why it matters
This patch fixes a bug where a single wallet command could freeze the wallet-rpc handling thread forever. The bug occurs when a user asks the wallet to estimate the size of a transaction with an extremely large number of outputs (more than about one billion). The old code used a signed integer in a loop that doubles a value until it is large enough; for very large inputs this overflows, behaves unpredictably, and never finishes, causing an infinite loop. The fix changes the calculation to use unsigned 64-bit integers, which cannot overflow in the same dangerous way, and also makes the size arithmetic safer for large counts.
Apply the patch. It is a minimal, targeted fix. Consider also adding an explicit upper-bound validation on n_outputs in estimate_tx_size_and_weight() and wallet-rpc endpoints to reject absurdly large values before they reach the estimator, providing defense in depth.
Security signals we found
Infinite loop / denial of service in wallet-rpc handling thread
Signed integer overflow in bit-shift loop
Unbounded n_outputs accepted by estimate_tx_size_and_weight
Size arithmetic overflow risk for large input/output counts
Single RPC call can freeze wallet-rpc service
Evidence from the diff
The functions estimate_rct_tx_size() and estimate_tx_weight() padded the Bulletproof output count using while ((1<<log_padded_outputs) < n_outputs), where 1 is a signed int. For n_outputs > 2^30, the shift reaches 1<<31 (signed overflow, UB), and once the shift count exceeds the int width the result cycles and never reaches n_outputs, producing an infinite loop. estimate_tx_size_and_weight() only rejects negative n_outputs, so a wallet-rpc caller can pass INT_MAX and spin the handling thread. The patch replaces the signed shifts with UINT64_C(1) << log_padded_outputs and compares against (uint64_t)n_outputs, and casts per-input/per-output terms to size_t to prevent size arithmetic overflow.
Changed components
src/wallet/wallet2.cppestimate_rct_tx_size()estimate_tx_weight()estimate_tx_size()wallet-rpc transaction size estimationInspect captured patch +13 / −13
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index d4b3754..d6a830e 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -806,10 +806,10 @@ size_t estimate_rct_tx_size(int n_inputs, int mixin, int n_outputs, size_t extra
size += 1 + 6;
// vin
- size += n_inputs * (1+6+(mixin+1)*2+32);
+ size += (size_t)n_inputs * (1+6+((size_t)mixin+1)*2+32);
// vout
- size += n_outputs * (6+32);
+ size += (size_t)n_outputs * (6+32);
// extra
size += extra_size;
@@ -823,31 +823,31 @@ size_t estimate_rct_tx_size(int n_inputs, int mixin, int n_outputs, size_t extra
if (bulletproof || bulletproof_plus)
{
size_t log_padded_outputs = 0;
- while ((1<<log_padded_outputs) < n_outputs)
+ while ((UINT64_C(1) << log_padded_outputs) < (uint64_t)n_outputs)
++log_padded_outputs;
size += (2 * (6 + log_padded_outputs) + (bulletproof_plus ? 6 : (4 + 5))) * 32 + 3;
}
else
- size += (2*64*32+32+64*32) * n_outputs;
+ size += (size_t)(2*64*32+32+64*32) * n_outputs;
// MGs/CLSAGs
if (clsag)
- size += n_inputs * (32 * (mixin+1) + 64);
+ size += (size_t)n_inputs * (32 * ((size_t)mixin+1) + 64);
else
- size += n_inputs * (64 * (mixin+1) + 32);
+ size += (size_t)n_inputs * (64 * ((size_t)mixin+1) + 32);
if (use_view_tags)
- size += n_outputs * sizeof(crypto::view_tag);
+ size += (size_t)n_outputs * sizeof(crypto::view_tag);
// mixRing - not serialized, can be reconstructed
/* size += 2 * 32 * (mixin+1) * n_inputs; */
// pseudoOuts
- size += 32 * n_inputs;
+ size += (size_t)32 * n_inputs;
// ecdhInfo
- size += 8 * n_outputs;
+ size += (size_t)8 * n_outputs;
// outPk - only commitment is saved
- size += 32 * n_outputs;
+ size += (size_t)32 * n_outputs;
// txnFee
size += 4;
@@ -860,7 +860,7 @@ size_t estimate_tx_size(bool use_rct, int n_inputs, int mixin, int n_outputs, si
if (use_rct)
return estimate_rct_tx_size(n_inputs, mixin, n_outputs, extra_size, bulletproof, clsag, bulletproof_plus, use_view_tags);
else
- return n_inputs * (mixin+1) * APPROXIMATE_INPUT_BYTES + extra_size + (use_view_tags ? (n_outputs * sizeof(crypto::view_tag)) : 0);
+ return (size_t)n_inputs * ((size_t)mixin+1) * APPROXIMATE_INPUT_BYTES + extra_size + (use_view_tags ? ((size_t)n_outputs * sizeof(crypto::view_tag)) : 0);
}
uint64_t estimate_tx_weight(bool use_rct, int n_inputs, int mixin, int n_outputs, size_t extra_size, bool bulletproof, bool clsag, bool bulletproof_plus, bool use_view_tags)
@@ -870,11 +870,11 @@ uint64_t estimate_tx_weight(bool use_rct, int n_inputs, int mixin, int n_outputs
{
const uint64_t bp_base = (32 * ((bulletproof_plus ? 6 : 9) + 7 * 2)) / 2; // notional size of a 2 output proof, normalized to 1 proof (ie, divided by 2)
size_t log_padded_outputs = 2;
- while ((1<<log_padded_outputs) < n_outputs)
+ while ((UINT64_C(1) << log_padded_outputs) < (uint64_t)n_outputs)
++log_padded_outputs;
uint64_t nlr = 2 * (6 + log_padded_outputs);
const uint64_t bp_size = 32 * ((bulletproof_plus ? 6 : 9) + nlr);
- const uint64_t bp_clawback = (bp_base * (1<<log_padded_outputs) - bp_size) * 4 / 5;
+ const uint64_t bp_clawback = (bp_base * (UINT64_C(1) << log_padded_outputs) - bp_size) * 4 / 5;
MDEBUG("clawback on size " << size << ": " << bp_clawback);
size += bp_clawback;
}
Why this scored 70/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.