mirror of
https://github.com/dashpay/dash.git
synced 2024-12-26 04:22:55 +01:00
4f632952bf
e980214bc4fd49530e8d50fe0a6657b8583bc6b5 serialization: prevent int overflow for big Coin::nHeight (pierrenn) Pull request description: This is an attempt to fix fuzzer issues 1,2,8 reported by practicalswift here : https://github.com/bitcoin/bitcoin/issues/18046 The fuzzer harness doesn't prevent deserialization of unrealistic high values for `Coin::nHeight`. In the [provided examples](https://github.com/bitcoin/bitcoin/issues/18046), we have : - `blockundo_deserialize` : the varint `0x8DD88DD700` is deserialized as `3944983552` in `Coin::nHeight` (`TxInOutFormatter::Unser`) - `coins_deserialize` : the varint `0x8DD5D5EC40` is deserialized as `3939874496` similarly - `txundo_deserialize`: the varint `0x8DCD828F01` is deserialized as `3921725441` in `Coin::nHeight` (`Coin::Unserialize`) Since `Coin::nHeight` is 31 bit long, multiplying a large value by 2 triggers the fuzzer. AFAIK those values are unrealistic (~70k years for the smallest..). I've looked a bit a reducing the range of values the fuzzer can deserialize, but this seems to be too much code change for not much. Hence this PR chooses to static cast `nHeight` when re-serializing; it seems to be the less intrusive/safest way to prevent the fuzzer output. Another more "upstream" approach would be to limit `Coin::nHeight` values to something more realistic, e.g. `0xFFFFFFF` (~5k years) :de3a30bab2/src/undo.h (L39)andde3a30bab2/src/coins.h (L71)Thanks ! NB: i was also not sure about the component/area to prefix the PR/commit with.. ? ACKs for top commit: practicalswift: ACK e980214bc4fd49530e8d50fe0a6657b8583bc6b5 -- patch looks correct promag: ACK e980214bc4fd49530e8d50fe0a6657b8583bc6b5. sipa: utACK e980214bc4fd49530e8d50fe0a6657b8583bc6b5 MarcoFalke: re-ACK e980214bc4fd49530e8d50fe0a6657b8583bc6b5 🎑 ryanofsky: Code review ACK e980214bc4fd49530e8d50fe0a6657b8583bc6b5. Just removed ternary ? 1 : 0 and replaced / 2 with >> 1 since last review Tree-SHA512: 905fc9e5e52a6857abee4a1c863751767835965804bb8c39474f27a120f65399ff4ba7a49ef1da0ba565379f8c12095bd384b6c492cf06776f01b2db68d522b8
72 lines
2.2 KiB
C++
72 lines
2.2 KiB
C++
// Copyright (c) 2009-2010 Satoshi Nakamoto
|
|
// Copyright (c) 2009-2014 The Bitcoin Core developers
|
|
// Distributed under the MIT software license, see the accompanying
|
|
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
|
|
|
#ifndef BITCOIN_UNDO_H
|
|
#define BITCOIN_UNDO_H
|
|
|
|
#include <coins.h>
|
|
#include <compressor.h>
|
|
#include <consensus/consensus.h>
|
|
#include <primitives/transaction.h>
|
|
#include <serialize.h>
|
|
#include <version.h>
|
|
|
|
/** Formatter for undo information for a CTxIn
|
|
*
|
|
* Contains the prevout's CTxOut being spent, and its metadata as well
|
|
* (coinbase or not, height). The serialization contains a dummy value of
|
|
* zero. This is compatible with older versions which expect to see
|
|
* the transaction version there.
|
|
*/
|
|
struct TxInUndoFormatter
|
|
{
|
|
template<typename Stream>
|
|
void Ser(Stream &s, const Coin& txout) {
|
|
::Serialize(s, VARINT(txout.nHeight * uint32_t{2} + txout.fCoinBase ));
|
|
if (txout.nHeight > 0) {
|
|
// Required to maintain compatibility with older undo format.
|
|
::Serialize(s, (unsigned char)0);
|
|
}
|
|
::Serialize(s, Using<TxOutCompression>(txout.out));
|
|
}
|
|
|
|
template<typename Stream>
|
|
void Unser(Stream &s, Coin& txout) {
|
|
uint32_t nCode = 0;
|
|
::Unserialize(s, VARINT(nCode));
|
|
txout.nHeight = nCode >> 1;
|
|
txout.fCoinBase = nCode & 1;
|
|
if (txout.nHeight > 0) {
|
|
// Old versions stored the version number for the last spend of
|
|
// a transaction's outputs. Non-final spends were indicated with
|
|
// height = 0.
|
|
unsigned int nVersionDummy;
|
|
::Unserialize(s, VARINT(nVersionDummy));
|
|
}
|
|
::Unserialize(s, Using<TxOutCompression>(txout.out));
|
|
}
|
|
};
|
|
|
|
/** Undo information for a CTransaction */
|
|
class CTxUndo
|
|
{
|
|
public:
|
|
// undo information for all txins
|
|
std::vector<Coin> vprevout;
|
|
|
|
SERIALIZE_METHODS(CTxUndo, obj) { READWRITE(Using<VectorFormatter<TxInUndoFormatter>>(obj.vprevout)); }
|
|
};
|
|
|
|
/** Undo information for a CBlock */
|
|
class CBlockUndo
|
|
{
|
|
public:
|
|
std::vector<CTxUndo> vtxundo; // for all but the coinbase
|
|
|
|
SERIALIZE_METHODS(CBlockUndo, obj) { READWRITE(obj.vtxundo); }
|
|
};
|
|
|
|
#endif // BITCOIN_UNDO_H
|