libwalletqt: fix potential out-of-bounds read
What the developer wrote
Authored by jpk68
libwalletqt: fix potential out-of-bounds read
What changed, and why it matters
This commit fixes a bug in the Monero GUI wallet where three functions that read transaction details could access memory beyond the bounds of an internal list when the list is empty. The old check used 'index > size - 1', which underflows when the list is empty (size 0), so the guard failed and the code could read from an invalid position. The new check 'index >= size' is safe even when the list is empty. This is a defensive fix that prevents a potential crash or reading of unintended data when the wallet processes an unsigned transaction with no outputs or fees recorded.
Apply the patch. It is a minimal, correct defensive fix. Consider adding unit tests that call amount(0), fee(0), and mixin(0) on an UnsignedTransaction with empty internal vectors to prevent regression. Review the codebase for the same 'index > container.size() - 1' pattern elsewhere.
Security signals we found
Out-of-bounds read due to unsigned integer underflow in bounds check
Classic 'index > size - 1' anti-pattern replaced with safe 'index >= size' check
Applies to transaction amount, fee, and mixin accessors in unsigned transaction handling
Potential crash or information disclosure when vector is empty and index is non-zero
Evidence from the diff
The patch changes three accessor methods in src/libwalletqt/UnsignedTransaction.cpp: amount(), fee(), and mixin(). Each copies a std::vector
Changed components
src/libwalletqt/UnsignedTransaction.cppUnsignedTransaction::amount(size_t)UnsignedTransaction::fee(size_t)UnsignedTransaction::mixin(size_t)Inspect captured patch +3 / −3
diff --git a/src/libwalletqt/UnsignedTransaction.cpp b/src/libwalletqt/UnsignedTransaction.cpp
index fcd4256..85f723b 100644
--- a/src/libwalletqt/UnsignedTransaction.cpp
+++ b/src/libwalletqt/UnsignedTransaction.cpp
@@ -43,7 +43,7 @@ QString UnsignedTransaction::errorString() const
quint64 UnsignedTransaction::amount(size_t index) const
{
std::vector<uint64_t> arr = m_pimpl->amount();
- if(index > arr.size() - 1)
+ if(index >= arr.size())
return 0;
return arr[index];
}
@@ -51,7 +51,7 @@ quint64 UnsignedTransaction::amount(size_t index) const
quint64 UnsignedTransaction::fee(size_t index) const
{
std::vector<uint64_t> arr = m_pimpl->fee();
- if(index > arr.size() - 1)
+ if(index >= arr.size())
return 0;
return arr[index];
}
@@ -59,7 +59,7 @@ quint64 UnsignedTransaction::fee(size_t index) const
quint64 UnsignedTransaction::mixin(size_t index) const
{
std::vector<uint64_t> arr = m_pimpl->mixin();
- if(index > arr.size() - 1)
+ if(index >= arr.size())
return 0;
return arr[index];
}
Why this scored 38/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.