From 115f9523fa940ac702b5ac6cd72b94fabcc3ad5c Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Fri, 5 Sep 2014 18:10:47 -0400 Subject: [PATCH 1/7] hex_to_pod needs the destination as an arg, as opposed to it returning the pod --- src/cryptonote_core/cryptonote_format_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/cryptonote_format_utils.cpp b/src/cryptonote_core/cryptonote_format_utils.cpp index 8b9f42376..057d11fba 100644 --- a/src/cryptonote_core/cryptonote_format_utils.cpp +++ b/src/cryptonote_core/cryptonote_format_utils.cpp @@ -635,7 +635,7 @@ namespace cryptonote if (string_tools::pod_to_hex(block_blob_hash) == correct_blob_hash_202612) { - res = string_tools::hex_to_pod(existing_block_id_202612); + string_tools::hex_to_pod(existing_block_id_202612, res); return true; } return get_object_hash(get_block_hashing_blob(b), res); From 9a16bb9936958c8d4fd8f6527fdcfebc2ca0c56b Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Fri, 5 Sep 2014 18:32:31 -0400 Subject: [PATCH 2/7] added double-check for 202612 block id if a new block has the same block id as 202612 but the wrong blobdata, this will tell the caller that the block id is actually null_hash rather than the 202612 block id. --- src/cryptonote_core/cryptonote_format_utils.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/cryptonote_core/cryptonote_format_utils.cpp b/src/cryptonote_core/cryptonote_format_utils.cpp index 057d11fba..c6e619bf4 100644 --- a/src/cryptonote_core/cryptonote_format_utils.cpp +++ b/src/cryptonote_core/cryptonote_format_utils.cpp @@ -638,7 +638,19 @@ namespace cryptonote string_tools::hex_to_pod(existing_block_id_202612, res); return true; } - return get_object_hash(get_block_hashing_blob(b), res); + bool hash_result = get_object_hash(get_block_hashing_blob(b), res); + + if (hash_result) + { + // make sure that we aren't looking at a block with the 202612 block id but not the correct blobdata + if (string_tools::pod_to_hex(res) == existing_block_id_202612) + { + LOG_ERROR("Block with block id for 202612 but incorrect block blob hash found!"); + res = null_hash; + return false; + } + } + return hash_result; } //--------------------------------------------------------------- crypto::hash get_block_hash(const block& b) From c05489938f2735efe1539cfab77d28e4ef2baa48 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Fri, 5 Sep 2014 22:04:11 -0400 Subject: [PATCH 3/7] override for get_block_longhash for block 202612 --- src/cryptonote_core/cryptonote_format_utils.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/cryptonote_core/cryptonote_format_utils.cpp b/src/cryptonote_core/cryptonote_format_utils.cpp index c6e619bf4..d023561c3 100644 --- a/src/cryptonote_core/cryptonote_format_utils.cpp +++ b/src/cryptonote_core/cryptonote_format_utils.cpp @@ -689,6 +689,13 @@ namespace cryptonote //--------------------------------------------------------------- bool get_block_longhash(const block& b, crypto::hash& res, uint64_t height) { + // block 202612 bug workaround + const std::string longhash_202612 = "84f64766475d51837ac9efbef1926486e58563c95a19fef4aec3254f03000000"; + if (height == 202612) + { + string_tools::hex_to_pod(longhash_202612, res); + return true; + } block b_local = b; //workaround to avoid const errors with do_serialize blobdata bd = get_block_hashing_blob(b); crypto::cn_slow_hash(bd.data(), bd.size(), res); From 2ef0aee81d20c002ed50d6dec4baceee1ac40b44 Mon Sep 17 00:00:00 2001 From: rfree2monero Date: Thu, 4 Sep 2014 15:17:54 +0200 Subject: [PATCH 4/7] Fix tree-hash cnt n^2. Asserts, comment. Squash2 --- src/crypto/tree-hash.c | 54 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/src/crypto/tree-hash.c b/src/crypto/tree-hash.c index a1c21d72b..573f44643 100644 --- a/src/crypto/tree-hash.c +++ b/src/crypto/tree-hash.c @@ -35,7 +35,43 @@ #include "hash-ops.h" +/// Quick check if this is power of two (use on unsigned types; in this case for size_t only) +bool ispowerof2_size_t(size_t x) { + return x && !(x & (x - 1)); +} + +/*** +* Round to power of two, for count>=3 and for count being not too large (as reasonable for tree hash calculations) +*/ +size_t tree_hash_cnt(size_t count) { + assert( count >= 3); // cases for 0,1,2 are handled elsewhere + // Round down the count size: fun(2**n)= 2**(n-1) to round down to power of two + size_t tmp = count - 1; + size_t jj = 1; + for (jj=1 ; tmp != 0 ; ++jj) { + tmp /= 2; // dividing by 2 until to get how many powers of 2 fits size_to tmp + } + size_t cnt = 1 << (jj-2); // cnt is the count, but rounded down to power of two + // printf("count=%zu cnt=%zu jj=%zu tmp=%zu \n" , count,cnt,jj,tmp); + assert( cnt > 0 ); assert( cnt >= count/2 ); assert( cnt <= count ); + assert( ispowerof2_size_t( cnt )); + return cnt; +} + void tree_hash(const char (*hashes)[HASH_SIZE], size_t count, char *root_hash) { +// The blockchain block at height 202612 http://monerochain.info/block/bbd604d2ba11ba27935e006ed39c9bfdd99b76bf4a50654bc1e1e61217962698 +// contained 514 transactions, that triggered bad calculation of variable "cnt" in the original version of this function +// as from CryptoNote code. +// +// This bug applies to all CN altcoins. +// +// Mathematical bug here was first published on 14:45:34 (GMT+2) 2014-09-04 by Rafal Freeman +// https://github.com/rfree2monero/bitmonero/commit/b417abfb7a297d09f1bbb6de29030f8de9952ac8 +// and soon also applied to CryptoNote (15:10 GMT+2), and BoolBerry used not fully correct work around: +// the work around of sizeof(size_t)*8 or <<3 as used before in 2 coins and in BBL later was blocking +// exploitation on normal platforms, how ever we strongly recommend the following fix because it removes +// mistake in mathematical formula. + assert(count > 0); if (count == 1) { memcpy(root_hash, hashes, HASH_SIZE); @@ -43,24 +79,30 @@ void tree_hash(const char (*hashes)[HASH_SIZE], size_t count, char *root_hash) { cn_fast_hash(hashes, 2 * HASH_SIZE, root_hash); } else { size_t i, j; - size_t cnt = count - 1; + + size_t cnt = tree_hash_cnt( count ); + size_t max_size_t = (size_t) -1; // max allowed value of size_t + assert( cnt < max_size_t/2 ); // reasonable size to avoid any overflows. /2 is extra; Anyway should be limited much stronger by logical code + // as we have sane limits on transactions counts in blockchain rules + char (*ints)[HASH_SIZE]; - for (i = 1; i < sizeof(size_t); i <<= 1) { - cnt |= cnt >> i; - } - cnt &= ~(cnt >> 1); - ints = alloca(cnt * HASH_SIZE); + size_t ints_size = cnt * HASH_SIZE; + ints = alloca(ints_size); memset( ints , 0 , ints_size); // allocate, and zero out as extra protection for using uninitialized mem + memcpy(ints, hashes, (2 * cnt - count) * HASH_SIZE); + for (i = 2 * cnt - count, j = 2 * cnt - count; j < cnt; i += 2, ++j) { cn_fast_hash(hashes[i], 64, ints[j]); } assert(i == count); + while (cnt > 2) { cnt >>= 1; for (i = 0, j = 0; j < cnt; i += 2, ++j) { cn_fast_hash(ints[i], 64, ints[j]); } } + cn_fast_hash(ints[0], 64, root_hash); } } From 480cf9668f4c98f388511dce20bc6811fef19a29 Mon Sep 17 00:00:00 2001 From: iamsmooth Date: Sat, 6 Sep 2014 10:06:54 +0000 Subject: [PATCH 5/7] checkpoints on restore; currently fails on 212 checkpoint --- src/cryptonote_core/blockchain_storage.cpp | 22 ++++++++++++++++++++-- src/cryptonote_core/checkpoints_create.h | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_core/blockchain_storage.cpp b/src/cryptonote_core/blockchain_storage.cpp index e47c35ea5..932733c3b 100644 --- a/src/cryptonote_core/blockchain_storage.cpp +++ b/src/cryptonote_core/blockchain_storage.cpp @@ -88,7 +88,25 @@ bool blockchain_storage::init(const std::string& config_folder) m_config_folder = config_folder; LOG_PRINT_L0("Loading blockchain..."); const std::string filename = m_config_folder + "/" CRYPTONOTE_BLOCKCHAINDATA_FILENAME; - if(!tools::unserialize_obj_from_file(*this, filename)) + if(tools::unserialize_obj_from_file(*this, filename)) + { + + // checkpoints + + // mainchain + for (size_t height=0; height < m_blocks.size(); ++height) + { + CHECK_AND_ASSERT_MES(m_checkpoints.is_in_checkpoint_zone(height) && m_checkpoints.check_block(height,get_block_hash(m_blocks[height].bl)),false,"checkpoint fail, blockchain.bin invalid"); + } + + // check alt chains + BOOST_FOREACH(blocks_ext_by_hash::value_type& alt_block, m_alternative_chains) + { + CHECK_AND_ASSERT_MES(m_checkpoints.is_alternative_block_allowed(m_blocks.size()-1,alt_block.second.height),false,"stored alternative block not allowed, blockchain.bin invalid"); + } + + } + else { LOG_PRINT_L0("Can't load blockchain storage from file, generating genesis block."); block bl = boost::value_initialized(); @@ -743,7 +761,7 @@ bool blockchain_storage::handle_alternative_block(const block& b, const crypto:: return false; } - //block is not related with head of main chain + //Block is not related with head of main chain //first of all - look in alternative chains container auto it_main_prev = m_blocks_index.find(b.prev_id); auto it_prev = m_alternative_chains.find(b.prev_id); diff --git a/src/cryptonote_core/checkpoints_create.h b/src/cryptonote_core/checkpoints_create.h index 932f31da0..fe77ccd0d 100644 --- a/src/cryptonote_core/checkpoints_create.h +++ b/src/cryptonote_core/checkpoints_create.h @@ -42,6 +42,10 @@ namespace cryptonote { ADD_CHECKPOINT(29556, "53c484a8ed91e4da621bb2fa88106dbde426fe90d7ef07b9c1e5127fb6f3a7f6"); ADD_CHECKPOINT(50000, "0fe8758ab06a8b9cb35b7328fd4f757af530a5d37759f9d3e421023231f7b31c"); ADD_CHECKPOINT(80000, "a62dcd7b536f22e003ebae8726e9e7276f63d594e264b6f0cd7aab27b66e75e3"); + ADD_CHECKPOINT(202212, "bbd604d2ba11ba27935e006ed39c9bfdd99b76bf4a50654bc1e1e61217962698"); + ADD_CHECKPOINT(202213, "44c921da50148949853df082c16f5f8e603ab2f9d599c8aee0f0a3f4abc4fb09"); // from chainradar; verify this + ADD_CHECKPOINT(202214, "043ce17bab57777d04423a65fd60487d02750cbdef11192dca97f1fa6dae654c"); // from chainradar; verify this + ADD_CHECKPOINT(205000, "5d3d7a26e6dc7535e34f03def711daa8c263785f73ec1fadef8a45880fde8063"); // from chainradar; verify this return true; } } From 07680e553fdaec1d05e2e56a97f9dc16a45b4768 Mon Sep 17 00:00:00 2001 From: iamsmooth Date: Sat, 6 Sep 2014 10:35:09 +0000 Subject: [PATCH 6/7] bug fix to checkpoint-on-restore; still fails on 612 --- src/cryptonote_core/blockchain_storage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/blockchain_storage.cpp b/src/cryptonote_core/blockchain_storage.cpp index 932733c3b..a3c68d64e 100644 --- a/src/cryptonote_core/blockchain_storage.cpp +++ b/src/cryptonote_core/blockchain_storage.cpp @@ -96,7 +96,7 @@ bool blockchain_storage::init(const std::string& config_folder) // mainchain for (size_t height=0; height < m_blocks.size(); ++height) { - CHECK_AND_ASSERT_MES(m_checkpoints.is_in_checkpoint_zone(height) && m_checkpoints.check_block(height,get_block_hash(m_blocks[height].bl)),false,"checkpoint fail, blockchain.bin invalid"); + CHECK_AND_ASSERT_MES((!m_checkpoints.is_in_checkpoint_zone(height)) || m_checkpoints.check_block(height,get_block_hash(m_blocks[height].bl)),false,"checkpoint fail, blockchain.bin invalid"); } // check alt chains From 0a9f2f5236f4679209db2c4ee5eb4e223f619d24 Mon Sep 17 00:00:00 2001 From: iamsmooth Date: Sat, 6 Sep 2014 17:59:24 +0000 Subject: [PATCH 7/7] fix checkpoints --- src/cryptonote_core/blockchain_storage.cpp | 5 ++++- src/cryptonote_core/checkpoints_create.h | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/cryptonote_core/blockchain_storage.cpp b/src/cryptonote_core/blockchain_storage.cpp index a3c68d64e..c41b50204 100644 --- a/src/cryptonote_core/blockchain_storage.cpp +++ b/src/cryptonote_core/blockchain_storage.cpp @@ -100,11 +100,14 @@ bool blockchain_storage::init(const std::string& config_folder) } // check alt chains + #if 0 + // doesn't work when a checkpoint is added and there are already alt chains. However, the rest of the blockchain code suffers from the same issue, so ignore for now + // see issue #118 BOOST_FOREACH(blocks_ext_by_hash::value_type& alt_block, m_alternative_chains) { CHECK_AND_ASSERT_MES(m_checkpoints.is_alternative_block_allowed(m_blocks.size()-1,alt_block.second.height),false,"stored alternative block not allowed, blockchain.bin invalid"); } - + #endif } else { diff --git a/src/cryptonote_core/checkpoints_create.h b/src/cryptonote_core/checkpoints_create.h index fe77ccd0d..9d65dc243 100644 --- a/src/cryptonote_core/checkpoints_create.h +++ b/src/cryptonote_core/checkpoints_create.h @@ -42,9 +42,9 @@ namespace cryptonote { ADD_CHECKPOINT(29556, "53c484a8ed91e4da621bb2fa88106dbde426fe90d7ef07b9c1e5127fb6f3a7f6"); ADD_CHECKPOINT(50000, "0fe8758ab06a8b9cb35b7328fd4f757af530a5d37759f9d3e421023231f7b31c"); ADD_CHECKPOINT(80000, "a62dcd7b536f22e003ebae8726e9e7276f63d594e264b6f0cd7aab27b66e75e3"); - ADD_CHECKPOINT(202212, "bbd604d2ba11ba27935e006ed39c9bfdd99b76bf4a50654bc1e1e61217962698"); - ADD_CHECKPOINT(202213, "44c921da50148949853df082c16f5f8e603ab2f9d599c8aee0f0a3f4abc4fb09"); // from chainradar; verify this - ADD_CHECKPOINT(202214, "043ce17bab57777d04423a65fd60487d02750cbdef11192dca97f1fa6dae654c"); // from chainradar; verify this + ADD_CHECKPOINT(202612, "bbd604d2ba11ba27935e006ed39c9bfdd99b76bf4a50654bc1e1e61217962698"); + ADD_CHECKPOINT(202613, "e2aa337e78df1f98f462b3b1e560c6b914dec47b610698b7b7d1e3e86b6197c2"); // from chainradar; verify this + ADD_CHECKPOINT(202614, "c29e3dc37d8da3e72e506e31a213a58771b24450144305bcba9e70fa4d6ea6fb"); // from chainradar; verify this ADD_CHECKPOINT(205000, "5d3d7a26e6dc7535e34f03def711daa8c263785f73ec1fadef8a45880fde8063"); // from chainradar; verify this return true; }