low risk, potentially varint overflow bug patched thanks to BBR

This commit is contained in:
Riccardo Spagni 2014-09-24 22:28:25 +02:00 committed by Thomas Winget
parent 5cd77a9f0b
commit 4e2b2b942d
9 changed files with 85 additions and 14 deletions

View File

@ -36,7 +36,7 @@
#include <sstream> #include <sstream>
#include <string> #include <string>
/*! \file varint.h /*! \file varint.h
* \breif provides the implementation of varint's * \brief provides the implementation of varint's
* *
* The representation of varints is rather odd. The first bit of each * The representation of varints is rather odd. The first bit of each
* octet is significant, it represents wheter there is another part * octet is significant, it represents wheter there is another part
@ -52,6 +52,29 @@
namespace tools { namespace tools {
template<typename T>
size_t get_varint_packed_size(T v)
{
if(v <= 127)
return 1;
else if(v <= 16383)
return 2;
else if(v <= 2097151)
return 3;
else if(v <= 268435455)
return 4;
else if(v <= 34359738367)
return 5;
else if(v <= 4398046511103)
return 6;
else if(v <= 4398046511103)
return 6;
else if(v <= 562949953421311)
return 7;
else
return 8;
}
/*! \brief Error codes for varint /*! \brief Error codes for varint
*/ */
enum { enum {

View File

@ -226,7 +226,6 @@ namespace cryptonote
ar.end_array(); ar.end_array();
END_SERIALIZE() END_SERIALIZE()
private:
static size_t get_signature_size(const txin_v& tx_in); static size_t get_signature_size(const txin_v& tx_in);
}; };

View File

@ -191,7 +191,7 @@ namespace cryptonote
return false; return false;
} }
bool r = add_new_tx(tx, tx_hash, tx_prefixt_hash, tx_blob.size(), tvc, keeped_by_block); bool r = add_new_tx(tx, tx_hash, tx_prefixt_hash, tvc, keeped_by_block);
if(tvc.m_verifivation_failed) if(tvc.m_verifivation_failed)
{LOG_PRINT_RED_L1("Transaction verification failed: " << tx_hash);} {LOG_PRINT_RED_L1("Transaction verification failed: " << tx_hash);}
else if(tvc.m_verifivation_impossible) else if(tvc.m_verifivation_impossible)
@ -284,7 +284,7 @@ namespace cryptonote
crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx); crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx);
blobdata bl; blobdata bl;
t_serializable_object_to_blob(tx, bl); t_serializable_object_to_blob(tx, bl);
return add_new_tx(tx, tx_hash, tx_prefix_hash, bl.size(), tvc, keeped_by_block); return add_new_tx(tx, tx_hash, tx_prefix_hash, tvc, keeped_by_block);
} }
//----------------------------------------------------------------------------------------------- //-----------------------------------------------------------------------------------------------
size_t core::get_blockchain_total_transactions() size_t core::get_blockchain_total_transactions()
@ -297,7 +297,7 @@ namespace cryptonote
// return m_blockchain_storage.get_outs(amount, pkeys); // return m_blockchain_storage.get_outs(amount, pkeys);
//} //}
//----------------------------------------------------------------------------------------------- //-----------------------------------------------------------------------------------------------
bool core::add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block) bool core::add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, tx_verification_context& tvc, bool keeped_by_block)
{ {
if(m_mempool.have_tx(tx_hash)) if(m_mempool.have_tx(tx_hash))
{ {
@ -311,7 +311,7 @@ namespace cryptonote
return true; return true;
} }
return m_mempool.add_tx(tx, tx_hash, blob_size, tvc, keeped_by_block); return m_mempool.add_tx(tx, tx_hash, tvc, keeped_by_block);
} }
//----------------------------------------------------------------------------------------------- //-----------------------------------------------------------------------------------------------
bool core::get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, const blobdata& ex_nonce) bool core::get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, const blobdata& ex_nonce)

View File

@ -120,7 +120,7 @@ namespace cryptonote
uint64_t get_target_blockchain_height() const; uint64_t get_target_blockchain_height() const;
private: private:
bool add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block); bool add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, tx_verification_context& tvc, bool keeped_by_block);
bool add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block); bool add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block);
bool add_new_block(const block& b, block_verification_context& bvc); bool add_new_block(const block& b, block_verification_context& bvc);
bool load_state_data(); bool load_state_data();

View File

@ -742,6 +742,23 @@ namespace cryptonote
return true; return true;
} }
//--------------------------------------------------------------- //---------------------------------------------------------------
size_t get_object_blobsize(const transaction& t)
{
size_t prefix_blob = get_object_blobsize(static_cast<const transaction_prefix&>(t));
if(is_coinbase(t))
return prefix_blob;
for(const auto& in: t.vin)
{
size_t sig_count = transaction::get_signature_size(in);
prefix_blob += 64*sig_count;
prefix_blob += tools::get_varint_packed_size(sig_count);
}
prefix_blob += tools::get_varint_packed_size(t.vin.size());
return prefix_blob;
}
//---------------------------------------------------------------
blobdata block_to_blob(const block& b) blobdata block_to_blob(const block& b)
{ {
return t_serializable_object_to_blob(b); return t_serializable_object_to_blob(b);

View File

@ -157,6 +157,8 @@ namespace cryptonote
return b.size(); return b.size();
} }
//--------------------------------------------------------------- //---------------------------------------------------------------
size_t get_object_blobsize(const transaction& t);
//---------------------------------------------------------------
template<class t_object> template<class t_object>
bool get_object_hash(const t_object& o, crypto::hash& res, size_t& blob_size) bool get_object_hash(const t_object& o, crypto::hash& res, size_t& blob_size)
{ {

View File

@ -59,9 +59,9 @@ namespace cryptonote
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
bool tx_memory_pool::add_tx(const transaction &tx, /*const crypto::hash& tx_prefix_hash,*/ const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool kept_by_block) bool tx_memory_pool::add_tx(const transaction &tx, const crypto::hash &id, tx_verification_context& tvc, bool kept_by_block)
{ {
size_t blob_size = get_object_blobsize(tx);
if(!check_inputs_types_supported(tx)) if(!check_inputs_types_supported(tx))
{ {
@ -179,9 +179,8 @@ namespace cryptonote
bool tx_memory_pool::add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block) bool tx_memory_pool::add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block)
{ {
crypto::hash h = null_hash; crypto::hash h = null_hash;
size_t blob_size = 0; get_transaction_hash(tx, h);
get_transaction_hash(tx, h, blob_size); return add_tx(tx, h, tvc, keeped_by_block);
return add_tx(tx, h, blob_size, tvc, keeped_by_block);
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
bool tx_memory_pool::remove_transaction_keyimages(const transaction& tx) bool tx_memory_pool::remove_transaction_keyimages(const transaction& tx)

View File

@ -56,7 +56,7 @@ namespace cryptonote
{ {
public: public:
tx_memory_pool(blockchain_storage& bchs); tx_memory_pool(blockchain_storage& bchs);
bool add_tx(const transaction &tx, const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block); bool add_tx(const transaction &tx, const crypto::hash &id, tx_verification_context& tvc, bool keeped_by_block);
bool add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block); bool add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block);
//gets tx and remove it from pool //gets tx and remove it from pool
bool take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee); bool take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee);
@ -81,7 +81,7 @@ namespace cryptonote
/*bool flush_pool(const std::strig& folder); /*bool flush_pool(const std::strig& folder);
bool inflate_pool(const std::strig& folder);*/ bool inflate_pool(const std::strig& folder);*/
#define CURRENT_MEMPOOL_ARCHIVE_VER 8 #define CURRENT_MEMPOOL_ARCHIVE_VER 9
template<class archive_t> template<class archive_t>
void serialize(archive_t & a, const unsigned int version) void serialize(archive_t & a, const unsigned int version)

View File

@ -299,6 +299,37 @@ namespace
} }
} }
bool test_get_varint_packed_size_for_num(uint64_t n)
{
std::stringstream ss;
typedef std::ostreambuf_iterator<char> it;
tools::write_varint(it(ss), n);
uint64_t sz = ss.str().size();
if(sz != tools::get_varint_packed_size(n))
return false;
else
return true;
}
TEST(Serialization, validate_get_varint_packed_size)
{
ASSERT_TRUE(test_get_varint_packed_size_for_num(127));
ASSERT_TRUE(test_get_varint_packed_size_for_num(128));
ASSERT_TRUE(test_get_varint_packed_size_for_num(16383));
ASSERT_TRUE(test_get_varint_packed_size_for_num(16383+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(2097151));
ASSERT_TRUE(test_get_varint_packed_size_for_num(2097151+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(268435455));
ASSERT_TRUE(test_get_varint_packed_size_for_num(268435455+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(34359738367));
ASSERT_TRUE(test_get_varint_packed_size_for_num(34359738367+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(4398046511103));
ASSERT_TRUE(test_get_varint_packed_size_for_num(4398046511103+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(4398046511103));
ASSERT_TRUE(test_get_varint_packed_size_for_num(4398046511103+1));
ASSERT_TRUE(test_get_varint_packed_size_for_num(562949953421311));
ASSERT_TRUE(test_get_varint_packed_size_for_num(562949953421311+1));
}
TEST(Serialization, serializes_transacion_signatures_correctly) TEST(Serialization, serializes_transacion_signatures_correctly)
{ {
using namespace cryptonote; using namespace cryptonote;