sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-26 12:32:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #9753: Add static_assert to prevent VARINT(<signed value>)

Browse Source 
499d95e27 Add static_assert to prevent VARINT(<signed value>) (Russell Yanofsky)

Pull request description:

  Using VARINT with signed types is dangerous because negative values will appear to serialize correctly, but then deserialize as positive values mod 128.

  This commit changes the VARINT macro to trigger a compile error by default if called with an signed value, and it updates existing broken uses of VARINT to pass a special flag that lets them keep working with no changes in behavior.

  There is some discussion about this issue here: https://github.com/bitcoin/bitcoin/pull/9693#issuecomment-278701473. I think another good change along these lines would be to make `GetSizeOfVarInt` and `WriteVarInt` throw exceptions if they are passed numbers less than 0 to serialize. But unlike this change, that would be a change in runtime behavior, and need more consideration.

Tree-SHA512: 082c65598cfac6dc1da042bdb47dbc9d5d789fc849fe52921cc238578588f4e5ff976c8b4b2ce42cb75290eb14f3b42ea76e26202c223c5b2aa63ef45c2ea3cc
This commit is contained in:
Wladimir J. van der Laan 2020-12-17 15:20:31 +03:00 committed by UdjinM6
parent f8ac5a555b
commit 6ca09aaae3
No known key found for this signature in database
GPG Key ID: 83592BD1400D58D9
7 changed files with 62 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/chain.h
View File

@ -91,7 +91,7 @@ struct CDiskBlockPos
template <typename Stream, typename Operation> template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action) { inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(VARINT(nFile)); READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED));
READWRITE(VARINT(nPos)); READWRITE(VARINT(nPos));
} }
@ -389,13 +389,13 @@ public:
inline void SerializationOp(Stream& s, Operation ser_action) { inline void SerializationOp(Stream& s, Operation ser_action) {
int _nVersion = s.GetVersion(); int _nVersion = s.GetVersion();
if (!(s.GetType() & SER_GETHASH)) if (!(s.GetType() & SER_GETHASH))
READWRITE(VARINT(_nVersion)); READWRITE(VARINT(_nVersion, VarIntMode::NONNEGATIVE_SIGNED));
READWRITE(VARINT(nHeight)); READWRITE(VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED));
READWRITE(VARINT(nStatus)); READWRITE(VARINT(nStatus));
READWRITE(VARINT(nTx)); READWRITE(VARINT(nTx));
if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO))
READWRITE(VARINT(nFile)); READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED));
if (nStatus & BLOCK_HAVE_DATA) if (nStatus & BLOCK_HAVE_DATA)
READWRITE(VARINT(nDataPos)); READWRITE(VARINT(nDataPos));
if (nStatus & BLOCK_HAVE_UNDO) if (nStatus & BLOCK_HAVE_UNDO)

8
src/evo/deterministicmns.h
View File

@ -579,12 +579,12 @@ public:
s << addedMNs; s << addedMNs;
WriteCompactSize(s, updatedMNs.size()); WriteCompactSize(s, updatedMNs.size());
for (const auto& p : updatedMNs) { for (const auto& p : updatedMNs) {
WriteVarInt(s, p.first); WriteVarInt<Stream, VarIntMode::DEFAULT, uint64_t>(s, p.first);
s << p.second; s << p.second;
} }
WriteCompactSize(s, removedMns.size()); WriteCompactSize(s, removedMns.size());
for (const auto& p : removedMns) { for (const auto& p : removedMns) {
WriteVarInt(s, p); WriteVarInt<Stream, VarIntMode::DEFAULT, uint64_t>(s, p);
} }
} }
@ -600,13 +600,13 @@ public:
tmp = ReadCompactSize(s); tmp = ReadCompactSize(s);
for (size_t i = 0; i < tmp; i++) { for (size_t i = 0; i < tmp; i++) {
CDeterministicMNStateDiff diff; CDeterministicMNStateDiff diff;
tmp2 = ReadVarInt<Stream, uint64_t>(s); tmp2 = ReadVarInt<Stream, VarIntMode::DEFAULT, uint64_t>(s);
s >> diff; s >> diff;
updatedMNs.emplace(tmp2, std::move(diff)); updatedMNs.emplace(tmp2, std::move(diff));
} }
tmp = ReadCompactSize(s); tmp = ReadCompactSize(s);
for (size_t i = 0; i < tmp; i++) { for (size_t i = 0; i < tmp; i++) {
tmp2 = ReadVarInt<Stream, uint64_t>(s); tmp2 = ReadVarInt<Stream, VarIntMode::DEFAULT, uint64_t>(s);
removedMns.emplace(tmp2); removedMns.emplace(tmp2);
} }
} }

3
src/node/coinstats.cpp
View File

@ -29,8 +29,7 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash,
for (const auto& output : outputs) { for (const auto& output : outputs) {
ss << VARINT(output.first + 1); ss << VARINT(output.first + 1);
ss << output.second.out.scriptPubKey; ss << output.second.out.scriptPubKey;
// ss << VARINT(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED); ss << VARINT(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED);
ss << VARINT(output.second.out.nValue);
stats.nTransactionOutputs++; stats.nTransactionOutputs++;
stats.nTotalAmount += output.second.out.nValue; stats.nTotalAmount += output.second.out.nValue;
stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ + stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ +

48
src/serialize.h
View File

@ -346,9 +346,31 @@ uint64_t ReadCompactSize(Stream& is)
* 2^32: [0x8E 0xFE 0xFE 0xFF 0x00] * 2^32: [0x8E 0xFE 0xFE 0xFF 0x00]
*/ */
template<typename I> /**
* Mode for encoding VarInts.
*
* Currently there is no support for signed encodings. The default mode will not
* compile with signed values, and the legacy "nonnegative signed" mode will
* accept signed values, but improperly encode and decode them if they are
* negative. In the future, the DEFAULT mode could be extended to support
* negative numbers in a backwards compatible way, and additional modes could be
* added to support different varint formats (e.g. zigzag encoding).
*/
enum class VarIntMode { DEFAULT, NONNEGATIVE_SIGNED };
template <VarIntMode Mode, typename I>
struct CheckVarIntMode {
constexpr CheckVarIntMode()
{
static_assert(Mode != VarIntMode::DEFAULT || std::is_unsigned<I>::value, "Unsigned type required with mode DEFAULT.");
static_assert(Mode != VarIntMode::NONNEGATIVE_SIGNED || std::is_signed<I>::value, "Signed type required with mode NONNEGATIVE_SIGNED.");
}
};
template<VarIntMode Mode, typename I>
inline unsigned int GetSizeOfVarInt(I n) inline unsigned int GetSizeOfVarInt(I n)
{ {
CheckVarIntMode<Mode, I>();
int nRet = 0; int nRet = 0;
while(true) { while(true) {
nRet++; nRet++;
@ -362,9 +384,10 @@ inline unsigned int GetSizeOfVarInt(I n)
template<typename I> template<typename I>
inline void WriteVarInt(CSizeComputer& os, I n); inline void WriteVarInt(CSizeComputer& os, I n);
template<typename Stream, typename I> template<typename Stream, VarIntMode Mode, typename I>
void WriteVarInt(Stream& os, I n) void WriteVarInt(Stream& os, I n)
{ {
CheckVarIntMode<Mode, I>();
unsigned char tmp[(sizeof(n)*8+6)/7]; unsigned char tmp[(sizeof(n)*8+6)/7];
int len=0; int len=0;
while(true) { while(true) {
@ -379,9 +402,10 @@ void WriteVarInt(Stream& os, I n)
} while(len--); } while(len--);
} }
template<typename Stream, typename I> template<typename Stream, VarIntMode Mode, typename I>
I ReadVarInt(Stream& is) I ReadVarInt(Stream& is)
{ {
CheckVarIntMode<Mode, I>();
I n = 0; I n = 0;
while(true) { while(true) {
unsigned char chData = ser_readdata8(is); unsigned char chData = ser_readdata8(is);
@ -404,7 +428,7 @@ I ReadVarInt(Stream& is)
#define DYNBITSET(obj) CDynamicBitSet(REF(obj)) #define DYNBITSET(obj) CDynamicBitSet(REF(obj))
#define FIXEDVARINTSBITSET(obj, size) CFixedVarIntsBitSet(REF(obj), (size)) #define FIXEDVARINTSBITSET(obj, size) CFixedVarIntsBitSet(REF(obj), (size))
#define AUTOBITSET(obj, size) CAutoBitSet(REF(obj), (size)) #define AUTOBITSET(obj, size) CAutoBitSet(REF(obj), (size))
#define VARINT(obj) WrapVarInt(REF(obj)) #define VARINT(obj, ...) WrapVarInt<__VA_ARGS__>(REF(obj))
#define COMPACTSIZE(obj) CCompactSize(REF(obj)) #define COMPACTSIZE(obj) CCompactSize(REF(obj))
#define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj)) #define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj))
@ -488,11 +512,11 @@ public:
int32_t last = -1; int32_t last = -1;
for (int32_t i = 0; i < (int32_t)vec.size(); i++) { for (int32_t i = 0; i < (int32_t)vec.size(); i++) {
if (vec[i]) { if (vec[i]) {
WriteVarInt<Stream, uint32_t>(s, (uint32_t)(i - last)); WriteVarInt<Stream, VarIntMode::DEFAULT, uint32_t>(s, (uint32_t)(i - last));
last = i; last = i;
} }
} }
WriteVarInt(s, 0); // stopper WriteVarInt<Stream, VarIntMode::DEFAULT, uint32_t>(s, 0); // stopper
} }
template<typename Stream> template<typename Stream>
@ -502,7 +526,7 @@ public:
int32_t last = -1; int32_t last = -1;
while(true) { while(true) {
uint32_t offset = ReadVarInt<Stream, uint32_t>(s); uint32_t offset = ReadVarInt<Stream, VarIntMode::DEFAULT, uint32_t>(s);
if (offset == 0) { if (offset == 0) {
break; break;
} }
@ -564,7 +588,7 @@ public:
} }
}; };
template<typename I> template<VarIntMode Mode, typename I>
class CVarInt class CVarInt
{ {
protected: protected:
@ -574,12 +598,12 @@ public:
template<typename Stream> template<typename Stream>
void Serialize(Stream &s) const { void Serialize(Stream &s) const {
WriteVarInt<Stream,I>(s, n); WriteVarInt<Stream,Mode,I>(s, n);
} }
template<typename Stream> template<typename Stream>
void Unserialize(Stream& s) { void Unserialize(Stream& s) {
n = ReadVarInt<Stream,I>(s); n = ReadVarInt<Stream,Mode,I>(s);
} }
}; };
@ -668,8 +692,8 @@ public:
} }
}; };
template<typename I> template<VarIntMode Mode=VarIntMode::DEFAULT, typename I>
CVarInt<I> WrapVarInt(I& n) { return CVarInt<I>(n); } CVarInt<Mode, I> WrapVarInt(I& n) { return CVarInt<Mode, I>{n}; }
template<typename I> template<typename I>
BigEndian<I> WrapBigEndian(I& n) { return BigEndian<I>(n); } BigEndian<I> WrapBigEndian(I& n) { return BigEndian<I>(n); }

26
src/test/serialize_tests.cpp
View File

@ -181,8 +181,8 @@ BOOST_AUTO_TEST_CASE(varints)
CDataStream ss(SER_DISK, 0); CDataStream ss(SER_DISK, 0);
CDataStream::size_type size = 0; CDataStream::size_type size = 0;
for (int i = 0; i < 100000; i++) { for (int i = 0; i < 100000; i++) {
ss << VARINT(i); ss << VARINT(i, VarIntMode::NONNEGATIVE_SIGNED);
size += ::GetSerializeSize(VARINT(i), 0, 0); size += ::GetSerializeSize(VARINT(i, VarIntMode::NONNEGATIVE_SIGNED), 0, 0);
BOOST_CHECK(size == ss.size()); BOOST_CHECK(size == ss.size());
} }
@ -195,7 +195,7 @@ BOOST_AUTO_TEST_CASE(varints)
// decode // decode
for (int i = 0; i < 100000; i++) { for (int i = 0; i < 100000; i++) {
int j = -1; int j = -1;
ss >> VARINT(j); ss >> VARINT(j, VarIntMode::NONNEGATIVE_SIGNED);
BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i);
} }
@ -209,21 +209,21 @@ BOOST_AUTO_TEST_CASE(varints)
BOOST_AUTO_TEST_CASE(varints_bitpatterns) BOOST_AUTO_TEST_CASE(varints_bitpatterns)
{ {
CDataStream ss(SER_DISK, 0); CDataStream ss(SER_DISK, 0);
ss << VARINT(0); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); ss << VARINT(0, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear();
ss << VARINT(0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); ss << VARINT(0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear();
ss << VARINT((int8_t)0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); ss << VARINT((int8_t)0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear();
ss << VARINT(0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); ss << VARINT(0x80, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear();
ss << VARINT((uint8_t)0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); ss << VARINT((uint8_t)0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear();
ss << VARINT(0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); ss << VARINT(0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear();
ss << VARINT((int16_t)0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); ss << VARINT((int16_t)0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear();
ss << VARINT(0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); ss << VARINT(0xffff, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear();
ss << VARINT((uint16_t)0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); ss << VARINT((uint16_t)0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear();
ss << VARINT(0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); ss << VARINT(0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear();
ss << VARINT((int32_t)0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); ss << VARINT((int32_t)0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear();
ss << VARINT(0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT(0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear();
ss << VARINT((uint32_t)0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT((uint32_t)0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear();
ss << VARINT(0xffffffff); BOOST_CHECK_EQUAL(HexStr(ss), "8efefefe7f"); ss.clear(); ss << VARINT(0xffffffff); BOOST_CHECK_EQUAL(HexStr(ss), "8efefefe7f"); ss.clear();
ss << VARINT(0x7fffffffffffffffLL); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); ss << VARINT(0x7fffffffffffffffLL, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear();
ss << VARINT(0xffffffffffffffffULL); BOOST_CHECK_EQUAL(HexStr(ss), "80fefefefefefefefe7f"); ss.clear(); ss << VARINT(0xffffffffffffffffULL); BOOST_CHECK_EQUAL(HexStr(ss), "80fefefefefefefefe7f"); ss.clear();
} }

4
src/txdb.cpp
View File

@ -478,7 +478,7 @@ public:
void Unserialize(Stream &s) { void Unserialize(Stream &s) {
unsigned int nCode = 0; unsigned int nCode = 0;
// version // version
int nVersionDummy; unsigned int nVersionDummy;
::Unserialize(s, VARINT(nVersionDummy)); ::Unserialize(s, VARINT(nVersionDummy));
// header code // header code
::Unserialize(s, VARINT(nCode)); ::Unserialize(s, VARINT(nCode));
@ -505,7 +505,7 @@ public:
::Unserialize(s, CTxOutCompressor(vout[i])); ::Unserialize(s, CTxOutCompressor(vout[i]));
} }
// coinbase height // coinbase height
::Unserialize(s, VARINT(nHeight)); ::Unserialize(s, VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED));
} }
}; };

4
src/undo.h
View File

@ -26,7 +26,7 @@ class TxInUndoSerializer
public: public:
template<typename Stream> template<typename Stream>
void Serialize(Stream &s) const { void Serialize(Stream &s) const {
::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0))); ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0), VarIntMode::NONNEGATIVE_SIGNED));
if (txout->nHeight > 0) { if (txout->nHeight > 0) {
// Required to maintain compatibility with older undo format. // Required to maintain compatibility with older undo format.
::Serialize(s, (unsigned char)0); ::Serialize(s, (unsigned char)0);
@ -52,7 +52,7 @@ public:
// Old versions stored the version number for the last spend of // Old versions stored the version number for the last spend of
// a transaction's outputs. Non-final spends were indicated with // a transaction's outputs. Non-final spends were indicated with
// height = 0. // height = 0.
int nVersionDummy; unsigned int nVersionDummy;
::Unserialize(s, VARINT(nVersionDummy)); ::Unserialize(s, VARINT(nVersionDummy));
} }
::Unserialize(s, CTxOutCompressor(REF(txout->out))); ::Unserialize(s, CTxOutCompressor(REF(txout->out)));