Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#29176: wallet: Fix use-after-free in WalletBatc…
Browse files Browse the repository at this point in the history
…h::EraseRecords

faebf1d wallet: Fix use-after-free in WalletBatch::EraseRecords (MarcoFalke)

Pull request description:

  Creating a copy of the pointer to the underlying data of the stream is not enough to copy the data.

  Currently this happens to work sometimes, because the stream may not immediately free unused memory. However, there is no guarantee by the stream interface to always behave this way. Also, if `vector::clear` is called on the underlying memory, any pointers to it are invalid.

  Fix this, by creating a full copy of all bytes.

ACKs for top commit:
  achow101:
    ACK faebf1d

Tree-SHA512: 79ede9bc16cf257609545597bc6d9623ceead4531780ea6037cc5684aa3a7c7d80601354d315358defe47193f978a8ce40c5dc4637e32936c76157679b549ac5
  • Loading branch information
achow101 committed Jan 4, 2024
2 parents 65c05db + faebf1d commit d84f736
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/wallet/walletdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1401,13 +1401,13 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types)
}

// Make a copy of key to avoid data being deleted by the following read of the type
Span key_data{key};
const SerializeData key_data{key.begin(), key.end()};

std::string type;
key >> type;

if (types.count(type) > 0) {
if (!m_batch->Erase(key_data)) {
if (!m_batch->Erase(Span{key_data})) {
cursor.reset(nullptr);
m_batch->TxnAbort();
return false; // erase failed
Expand Down

0 comments on commit d84f736

Please sign in to comment.