Harden block validation and snapshot load paths

libraries/chain/CMakeLists.txt

@@ -91,6 +91,7 @@ add_library( sysio_chain
              resource_limits.cpp
              block_log.cpp
              block_root_processor.cpp
+             s_root_extension.cpp
              contract_action_match.cpp
              root_txn_identification.cpp
              transaction_context.cpp

libraries/chain/block_header.cpp

@@ -21,7 +21,10 @@ namespace sysio { namespace chain {
       // Do not include signed_block_header attributes in id, specifically exclude producer_signature.
       block_id_type result = digest(); //fc::sha256::hash(*static_cast<const block_header*>(this));
       result._hash[0] &= 0xffffffff00000000;
-      result._hash[0] += fc::endian_reverse_u32(block_num()); // store the block num in the ID, 160 bits is plenty for the hash
+      // Store the block num in the low 32 bits. Use |= rather than += to make it
+      // explicit that this is bit-insertion into pre-cleared bits — no carry into
+      // the hash bytes is possible regardless of future mask changes.
+      result._hash[0] |= fc::endian_reverse_u32(block_num());
       return result;
    }
 

libraries/chain/block_state.cpp

@@ -184,39 +184,102 @@ block_state::block_state(snapshot_detail::snapshot_block_state_v1&& sbs)
          .last_pending_finalizer_policy_digest = sbs.last_pending_finalizer_policy_digest,
          .last_pending_finalizer_policy_start_timestamp = sbs.last_pending_finalizer_policy_start_timestamp
       }
-   , strong_digest(compute_finality_digest())
-   , weak_digest(create_weak_digest(strong_digest))
-   , aggregating_qc(active_finalizer_policy,
-                    pending_finalizer_policy ? pending_finalizer_policy->second : finalizer_policy_ptr{}) // just in case we receive votes
+   // strong_digest / weak_digest / aggregating_qc are value-initialized here and
+   // assigned their real values in the body after null-guards. compute_finality_digest()
+   // and aggregating_qc_sig_t's ctor both dereference active_finalizer_policy; a
+   // tampered snapshot with null pointers would crash inside the member init list
+   // before we could SYS_ASSERT on them. Explicit {} forces value-init (matters for
+   // weak_digest_t = std::array<uint8_t,N>, whose default-init leaves bytes
+   // indeterminate).
+   , strong_digest{}
+   , weak_digest{}
+   , aggregating_qc{}
    , valid(std::move(sbs.valid))
 {
-   header_exts = header.validate_and_extract_header_extensions();
+   // fc::raw::unpack<shared_ptr<T>> permits null via a leading bool; a tampered
+   // snapshot can plant nulls in any of these slots. Null-check BEFORE any code
+   // dereferences them (compute_finality_digest / aggregating_qc construction /
+   // policy validate() calls all require non-null pointers). activated_protocol_features
+   // is reached through compute_base_digest() from compute_finality_digest() below.
+   SYS_ASSERT(activated_protocol_features, snapshot_exception, "activated_protocol_features must not be null");
+   SYS_ASSERT(active_finalizer_policy, snapshot_exception, "active_finalizer_policy must not be null");
+   SYS_ASSERT(active_proposer_policy, snapshot_exception, "active_proposer_policy must not be null");
+   if (pending_finalizer_policy) {
+      SYS_ASSERT(pending_finalizer_policy->second, snapshot_exception,
+                 "pending_finalizer_policy must not contain a null policy");
+   }
+   for (const auto& [_, pol] : proposed_finalizer_policies) {
+      SYS_ASSERT(pol, snapshot_exception, "proposed_finalizer_policies entry must not be null");
+   }
 
-   // Snapshot hardening: validate finality_core invariants
+   // Snapshot hardening: validate finality_core invariants BEFORE computing digests.
+   // compute_finality_digest() calls core.latest_qc_claim() (asserts !links.empty()) and
+   // core.get_block_reference() (asserts bounds on refs); an empty or malformed core
+   // would assert/segfault inside the digest path before we could throw snapshot_exception.
    core.validate_snapshot();
 
-   // Snapshot hardening: validate finalizer policies
-   SYS_ASSERT(active_finalizer_policy, snapshot_exception, "active_finalizer_policy must not be null");
-   SYS_ASSERT(active_proposer_policy, snapshot_exception, "active_proposer_policy must not be null");
-   active_finalizer_policy->validate_snapshot();
+   // Now safe to compute state that depends on active_finalizer_policy / activated_protocol_features / core.
+   strong_digest  = compute_finality_digest();
+   weak_digest    = create_weak_digest(strong_digest);
+   aggregating_qc = aggregating_qc_t{
+      active_finalizer_policy,
+      pending_finalizer_policy ? pending_finalizer_policy->second : finalizer_policy_ptr{}
+   };
+
+   header_exts = header.validate_and_extract_header_extensions();
+
+   // core.latest_qc_claim() is the authoritative reference for downstream QC-claim checks
+   // (see verify_basic_proper_block_invariants). For every non-genesis block it equals
+   // header.qc_claim by construction (finality_core::next sets latest_qc_claim from the
+   // header's claim). Genesis intentionally differs: core is {1, false}, header is {0, false}.
+   SYS_ASSERT(core.is_genesis_core() || core.latest_qc_claim() == header.qc_claim, snapshot_exception,
+              "snapshot block_state core.latest_qc_claim {} does not match header.qc_claim {}",
+              core.latest_qc_claim(), header.qc_claim);
+
+   // Snapshot hardening: validate finalizer and proposer policies.
+   // Uses the shared validate() methods (same checks the set_finalizers /
+   // set_proposed_producers intrinsics enforce); rewraps as snapshot_exception.
+   auto validate_fin_pol = [](const finalizer_policy& p, const char* which) {
+      try { p.validate(); }
+      catch (const invalid_finalizer_policy_exception& e) {
+         SYS_THROW(snapshot_exception, "snapshot: {} finalizer policy invalid: {}", which, e.top_message());
+      }
+   };
+   auto validate_prop_pol = [](const proposer_policy& p, const char* which) {
+      try { p.validate(); }
+      catch (const producer_schedule_exception& e) {
+         SYS_THROW(snapshot_exception, "snapshot: {} proposer policy invalid: {}", which, e.top_message());
+      }
+   };
+   validate_fin_pol(*active_finalizer_policy, "active");
    if (pending_finalizer_policy) {
-      pending_finalizer_policy->second->validate_snapshot();
+      validate_fin_pol(*pending_finalizer_policy->second, "pending");
    }
    for (const auto& [_, pol] : proposed_finalizer_policies) {
-      pol->validate_snapshot();
+      validate_fin_pol(*pol, "proposed");
    }
    if (latest_qc_claim_block_active_finalizer_policy) {
-      latest_qc_claim_block_active_finalizer_policy->validate_snapshot();
+      validate_fin_pol(*latest_qc_claim_block_active_finalizer_policy, "latest_qc_claim_block_active");
    }
-
-   // Snapshot hardening: validate valid_t
-   if (valid) {
-      valid->validation_tree.validate_snapshot();
-      auto expected_size = core.current_block_num() - core.last_final_block_num() + 1;
-      SYS_ASSERT(valid->validation_mroots.size() == expected_size, snapshot_exception,
-                 "valid_t.validation_mroots size ({}) != expected ({})",
-                 valid->validation_mroots.size(), expected_size);
+   validate_prop_pol(*active_proposer_policy, "active");
+   if (latest_proposed_proposer_policy) {
+      validate_prop_pol(*latest_proposed_proposer_policy, "latest_proposed");
    }
+   if (latest_pending_proposer_policy) {
+      validate_prop_pol(*latest_pending_proposer_policy, "latest_pending");
+   }
+
+   // Snapshot hardening: validate valid_t. A snapshot is never taken at genesis, so
+   // every snapshot block_state must carry a valid_t (finality-tree) structure. A null
+   // valid is a tampered or truncated snapshot; without this check get_validation_mroot()
+   // would later return an empty digest (after a debug-only assert) and the corruption
+   // would surface only indirectly as a finality_mroot mismatch in apply_block.
+   SYS_ASSERT(valid, snapshot_exception, "snapshot block_state must have a valid_t finality structure");
+   valid->validation_tree.validate_snapshot();
+   auto expected_size = core.current_block_num() - core.last_final_block_num() + 1;
+   SYS_ASSERT(valid->validation_mroots.size() == expected_size, snapshot_exception,
+              "valid_t.validation_mroots size ({}) != expected ({})",
+              valid->validation_mroots.size(), expected_size);
 }
 
 deque<transaction_metadata_ptr> block_state::extract_trxs_metas() {
@@ -302,6 +365,12 @@ valid_t block_state::new_valid(const block_header_state& next_bhs, const digest_
 
 digest_type block_state::get_validation_mroot(block_num_type target_block_num) const {
    if (!valid) {
+      // The only legitimate case where a block_state has no valid structure is
+      // the Savanna genesis core (created without a finality tree). Any other
+      // null-valid is a construction or serialization bug; catching it here
+      // produces a clearer failure than the downstream finality_mroot mismatch
+      // apply_block would report.
+      assert(core.is_genesis_core());
       return digest_type{};
    }
 

libraries/chain/controller.cpp

@@ -2459,6 +2459,13 @@ struct controller_impl {
    {
       const auto& pfs = protocol_features.get_protocol_feature_set();
 
+      // Track digests seen earlier in this activation list so duplicates fail here rather than
+      // later in start_block (which only catches duplicates of preactivated features). A duplicate
+      // that wasn't preactivated would otherwise re-run trigger_activation_handler and increment
+      // num_new_protocol_features_activated twice, diverging side effects from a legitimate block.
+      flat_set<digest_type> seen_in_this_activation;
+      seen_in_this_activation.reserve( new_protocol_features.size() );
+
       for( auto itr = new_protocol_features.begin(); itr != new_protocol_features.end(); ++itr ) {
          const auto& f = *itr;
 
@@ -2489,6 +2496,10 @@ struct controller_impl {
                      "protocol feature with digest '{}' has already been activated", f
          );
 
+         SYS_ASSERT( seen_in_this_activation.insert( f ).second, protocol_feature_exception,
+                     "protocol feature with digest '{}' appears more than once in the activation list", f
+         );
+
          auto dependency_checker = [&currently_activated_protocol_features, &new_protocol_features, &itr]
                                    ( const digest_type& f ) -> bool
          {
@@ -2667,43 +2678,7 @@ struct controller_impl {
             }
 
             if( are_multiple_state_roots_supported() ) {
-               // New: Process the s_header from block header extensions
-               auto rcvd_it = b->header_extensions.begin();
-               const auto next_rcvd = [&itr=rcvd_it, end=b->header_extensions.end()](bool include_start) {
-                  if( !include_start ) ++itr;
-                  // find the first s_root_extension in the received block header extensions
-                  return std::find_if(itr, end,
-                     [](const auto& ext) { return ext.first == s_root_extension::extension_id(); }); };
-               auto crtd_it = ab.header().header_extensions.begin();
-               const auto next_crtd = [&itr=crtd_it, end=ab.header().header_extensions.end()](bool include_start) {
-                  if( !include_start ) ++itr;
-                  // find the first s_root_extension in the locally constructed block header extensions
-                  // (which should be the same as the received block header extensions)
-                  return std::find_if(itr, end,
-                     [](const auto& ext) { return ext.first == s_root_extension::extension_id(); }); };
-               uint32_t count = 0;
-               bool rcvd = true;
-               bool crtd = true;
-               bool include_start = true;
-               while( rcvd ) {
-                  rcvd_it = next_rcvd(include_start);
-                  crtd_it = next_crtd(include_start);
-                  ++count;
-                  include_start = false; // only include start for the first iteration
-                  rcvd = rcvd_it != b->header_extensions.end();
-                  crtd = crtd_it != ab.header().header_extensions.end();
-                  SYS_ASSERT( rcvd == crtd, block_validate_exception,
-                              "The received block did{} have {} root header extensions, but the locally constructed one did{}",
-                              (rcvd ? "" : " not"), count, (crtd ? "" : " not") );
-                  if( rcvd && rcvd_it->second != crtd_it->second ) {
-                     s_header rcvd_s_header = fc::raw::unpack<s_header>(rcvd_it->second);
-                     s_header crtd_s_header = fc::raw::unpack<s_header>(crtd_it->second);
-                     SYS_THROW( block_validate_exception,
-                                "The received block root header extension, at slot number {}: {}; and the locally "
-                                "constructed one: {}; don't match!",
-                                count, rcvd_s_header.to_string(), crtd_s_header.to_string() );
-                  }
-               }
+               validate_s_root_extensions_match(b->header_extensions, ab.header().header_extensions);
             }
 
             if( !use_bsp_cached ) {
@@ -2798,9 +2773,6 @@ struct controller_impl {
    // -----------------------------------------------------------------------------
    std::optional<qc_t> verify_basic_proper_block_invariants( const block_id_type& id, const signed_block_ptr& b,
                                                              const block_state& prev ) {
-      if (prev.block_num() <= 1u)
-         return std::nullopt;
-
       SYS_ASSERT( b->block_extensions.empty(), invalid_block_extension, "No block extensions currently supported");
 
       const auto  new_qc_claim = b->qc_claim;
@@ -2816,7 +2788,12 @@ struct controller_impl {
          }
       }
 
-      const auto& prev_qc_claim = prev.header.qc_claim;
+      // Use prev.core.latest_qc_claim() rather than prev.header.qc_claim: for every non-genesis
+      // block they are equal by construction (finality_core::next sets latest_qc_claim from the
+      // header's claim), but genesis intentionally differs (core is {1, false}, header is {0, false}).
+      // The core is the authoritative source, so block 2 can validate cleanly with this reference.
+      // Snapshot-loaded block_state enforces this invariant in block_state(snapshot).
+      const qc_claim_t prev_qc_claim = prev.core.latest_qc_claim();
 
       // validate QC claim against previous block QC info
 
@@ -3261,7 +3238,7 @@ struct controller_impl {
    static checksum256_type calculate_trx_merkle( const deque<transaction_receipt>& trxs) {
       deque<digest_type> trx_digests;
       for( const auto& a : trxs ) {
-         trx_digests.emplace_back( (a.digest)() );
+         trx_digests.emplace_back( a.digest() );
       }
 
       return calc_merkle(std::move(trx_digests));

libraries/chain/finality_core.cpp

@@ -59,11 +59,15 @@ void finality_core::validate_snapshot() const {
    }
 
    // Invariant 8 (implied by 3-6): current_block_num - last_final_block_num == refs.size()
+   // Subtraction is safe: invariant 2 (checked above) established lfbn <= links.front().source_block_num,
+   // and by invariant 7's monotonicity links.front().source_block_num <= links.back().source_block_num == cur,
+   // so lfbn <= cur and the unsigned subtraction cannot underflow.
    SYS_ASSERT(cur - lfbn == refs.size(), snapshot_exception,
               "finality_core: current_block_num ({}) - last_final_block_num ({}) != refs.size() ({})",
               cur, lfbn, refs.size());
 
    // Invariant 9 (implied by 1,7): current_block_num - links.front().source_block_num == links.size() - 1
+   // Subtraction is safe by invariant 7's monotonicity (links.front().source_block_num <= cur).
    SYS_ASSERT(cur - links.front().source_block_num == links.size() - 1, snapshot_exception,
               "finality_core: link count mismatch: current_block_num ({}) - front source ({}) != links.size()-1 ({})",
               cur, links.front().source_block_num, links.size() - 1);

libraries/chain/fork_database.cpp

@@ -8,6 +8,7 @@
 #include <boost/multi_index/composite_key.hpp>
 #include <fc/io/fstream.hpp>
 #include <fc/io/cfile.hpp>
+#include <fmt/format.h>
 #include <fstream>
 #include <mutex>
 
@@ -26,12 +27,10 @@ namespace sysio::chain {
     */
 
    std::string log_fork_comparison(const block_state& bs) {
-      std::string r;
-      r += "[ latest_qc_block_timestamp: " + bs.latest_qc_block_timestamp().to_time_point().to_iso_string() + ", ";
-      r += "timestamp: " + bs.timestamp().to_time_point().to_iso_string() + ", ";
-      r += "id: " + bs.id().str();
-      r += " ]";
-      return r;
+      return fmt::format("[ latest_qc_block_timestamp: {}, timestamp: {}, id: {} ]",
+                         bs.latest_qc_block_timestamp().to_time_point().to_iso_string(),
+                         bs.timestamp().to_time_point().to_iso_string(),
+                         bs.id().str());
    }
 
    struct by_block_id;

libraries/chain/include/sysio/chain/exceptions.hpp

@@ -669,6 +669,8 @@ namespace sysio { namespace chain {
                                  3260000, "Finalizer exception" )
       FC_DECLARE_DERIVED_EXCEPTION( finalizer_safety_exception, finalizer_exception,
                                     3260001, "Finalizer safety file exception" )
+      FC_DECLARE_DERIVED_EXCEPTION( invalid_finalizer_policy_exception, finalizer_exception,
+                                    3260002, "Invalid finalizer policy" )
 
    /// @return true if exception requires transaction to be retried in the next block
    inline bool exception_is_exhausted(const fc::exception& e) {