wallet2: guard gamma picker against zero-output windows
What the developer wrote
Authored by selsta
wallet2: guard gamma picker against zero-output windows
What changed, and why it matters
This commit adds a safety check in Monero's wallet code to prevent a crash or undefined behavior when the wallet tries to select decoy transaction outputs but finds none in the expected time window. It also fixes a related unit test that was not correctly simulating a blockchain with only one output. The change is defensive and improves robustness, but the commit message does not describe it as a security fix.
Treat as a routine robustness fix. Review whether a zero-output window can occur in production (e.g., on a new or lightly used chain) and ensure the exception is handled gracefully by callers. No urgent security response is indicated by the available evidence.
Security signals we found
Division-by-zero guard added in wallet output selection
Defensive exception for empty consideration window
Unit test corrected to reflect cumulative offset semantics
Evidence from the diff
The gamma picker in wallet2.cpp computes outputs_to_consider over a window of recent blocks and then divides by it to get average_output_time. If outputs_to_consider is zero, this would cause a division-by-zero and likely crash or abnormal termination. The patch adds an explicit THROW_WALLET_EXCEPTION_IF(outputs_to_consider == 0, ...) guard. The unit test is corrected so cumulative offsets are realistic (all entries 1, not trailing zeros), and the test now loops up to 1000 tries to verify it can pick the single available output. The change is small and defensive; there is no direct evidence in the diff or commit metadata that this is an exploitable vulnerability.
Changed components
src/wallet/wallet2.cpptests/unit_tests/output_selection.cppInspect captured patch +11 / −4
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index c128eda..834679f 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -1051,6 +1051,7 @@ gamma_picker::gamma_picker(const std::vector<uint64_t> &rct_offsets, double shap
end = rct_offsets.data() + rct_offsets.size() - (std::max(1, CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE) - 1);
num_rct_outputs = *(end - 1);
THROW_WALLET_EXCEPTION_IF(num_rct_outputs == 0, error::wallet_internal_error, "No rct outputs");
+ THROW_WALLET_EXCEPTION_IF(outputs_to_consider == 0, error::wallet_internal_error, "No outputs in consideration window");
average_output_time = DIFFICULTY_TARGET_V2 * blocks_to_consider / static_cast<double>(outputs_to_consider); // this assumes constant target over the whole rct range
};
diff --git a/tests/unit_tests/output_selection.cpp b/tests/unit_tests/output_selection.cpp
index dd12d07..bd1eac6 100644
--- a/tests/unit_tests/output_selection.cpp
+++ b/tests/unit_tests/output_selection.cpp
@@ -261,16 +261,22 @@ TEST(select_outputs, exact_unlock_block)
TEST(select_outputs, exact_unlock_block_tiny)
{
- // Create chain of length CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE where there is one output in block 0
- std::vector<uint64_t> offsets(std::max(CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE, 1), 0);
- offsets[0] = 1;
+ // Create a chain of length CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE with one
+ // output in block 0. Since rct_offsets is cumulative, later blocks retain
+ // that output even though they contain no additional outputs.
+ std::vector<uint64_t> offsets(std::max(CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE, 1), 1);
tools::gamma_picker picker(offsets);
- constexpr size_t MAX_PICK_TRIES = 10;
+ constexpr size_t MAX_PICK_TRIES = 1000;
bool found_the_one_output = false;
for (size_t i = 0; i < MAX_PICK_TRIES; ++i)
+ {
if (picker.pick() == 0)
+ {
found_the_one_output = true;
+ break;
+ }
+ }
EXPECT_TRUE(found_the_one_output);
}
Why this scored 41/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.