Merge #19237: wallet: Check size after unserializing a pubkey
37ae687f95
Add tests for CPubKey serialization/unserialization (Elichai Turkel)9b8907fade
Check size after Unserializing CPubKey (Elichai Turkel) Pull request description: Found by practicalswift, closes #19235 Currently all the public API(except the pointer-like API) in CPubKey that sets/constructs a pubkey goes through `CPubKey::Set` which checks if that the length and size match and if not invalidates the key. This adds the same check to `CPubKey::Unserialize`, sadly I don't see an easy way to just push this to the existing checks in `CPubKey::Set` but it's only a simple condition. The problem with not invalidating is that if you write a pubkey like: `{0x02,0x00}` it will think the actual length is 33(because of `size()`) and will access uninitialized memory if you call any of the functions on CPubKey. ACKs for top commit: practicalswift: re-ACK37ae687f95
jonatack: Code review re-ACK37ae687
per `git diff eab8ee3 37ae687` only change since last review at eab8ee3 is passing the `pubkey` param by reference to const instead of by value in `src/test/key_tests.cpp::CmpSerializationPubkey` MarcoFalke: ACK37ae687f95
Tree-SHA512: 30173755555dfc76d6263fb6a59f41be36049ffae7b4e1b92b922d668f5e5e2331f7374d5fa10d5d59fc53020d2966156905ffcfa8b8129c1f6d0ca062174ff1
This commit is contained in:
commit
c9d1040d25
2 changed files with 47 additions and 0 deletions
|
@ -142,6 +142,9 @@ public:
|
|||
unsigned int len = ::ReadCompactSize(s);
|
||||
if (len <= SIZE) {
|
||||
s.read((char*)vch, len);
|
||||
if (len != size()) {
|
||||
Invalidate();
|
||||
}
|
||||
} else {
|
||||
// invalid pubkey, skip available data
|
||||
char dummy;
|
||||
|
|
|
@ -5,6 +5,7 @@
|
|||
#include <key.h>
|
||||
|
||||
#include <key_io.h>
|
||||
#include <streams.h>
|
||||
#include <test/util/setup_common.h>
|
||||
#include <uint256.h>
|
||||
#include <util/strencodings.h>
|
||||
|
@ -220,4 +221,47 @@ BOOST_AUTO_TEST_CASE(key_key_negation)
|
|||
BOOST_CHECK(key.GetPubKey().data()[0] == 0x03);
|
||||
}
|
||||
|
||||
static CPubKey UnserializePubkey(const std::vector<uint8_t>& data)
|
||||
{
|
||||
CDataStream stream{SER_NETWORK, INIT_PROTO_VERSION};
|
||||
stream << data;
|
||||
CPubKey pubkey;
|
||||
stream >> pubkey;
|
||||
return pubkey;
|
||||
}
|
||||
|
||||
static unsigned int GetLen(unsigned char chHeader)
|
||||
{
|
||||
if (chHeader == 2 || chHeader == 3)
|
||||
return CPubKey::COMPRESSED_SIZE;
|
||||
if (chHeader == 4 || chHeader == 6 || chHeader == 7)
|
||||
return CPubKey::SIZE;
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void CmpSerializationPubkey(const CPubKey& pubkey)
|
||||
{
|
||||
CDataStream stream{SER_NETWORK, INIT_PROTO_VERSION};
|
||||
stream << pubkey;
|
||||
CPubKey pubkey2;
|
||||
stream >> pubkey2;
|
||||
BOOST_CHECK(pubkey == pubkey2);
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(pubkey_unserialize)
|
||||
{
|
||||
for (uint8_t i = 2; i <= 7; ++i) {
|
||||
CPubKey key = UnserializePubkey({0x02});
|
||||
BOOST_CHECK(!key.IsValid());
|
||||
CmpSerializationPubkey(key);
|
||||
key = UnserializePubkey(std::vector<uint8_t>(GetLen(i), i));
|
||||
CmpSerializationPubkey(key);
|
||||
if (i == 5) {
|
||||
BOOST_CHECK(!key.IsValid());
|
||||
} else {
|
||||
BOOST_CHECK(key.IsValid());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
|
Loading…
Reference in a new issue