GH-1472 Initial impl of packed_transaction enforcement of extra data and no compression

Author: heifner

libraries/chain/controller.cpp

@@ -3832,6 +3832,8 @@ struct controller_impl {
 
             pending->_block_report.start_time = start;
 
+            const bool validate_ptrx = is_builtin_activated(builtin_protocol_feature_t::packed_transaction_restrictions);
+
             // validated in accept_block()
             std::get<building_block>(pending->_block_stage).trx_mroot_or_receipt_digests() = b->transaction_mroot;
 
@@ -3878,6 +3880,11 @@ struct controller_impl {
                                                          : (!!std::get<0>(trx_metas.at(packed_idx))
                                                                ? std::get<0>(trx_metas.at(packed_idx))
                                                                : std::get<1>(trx_metas.at(packed_idx)).get()));
+                  if (validate_ptrx) {
+                     const auto& ptrx = trx_meta->packed_trx();
+                     ptrx->validate_no_extra_data();
+                     ptrx->validate_no_compression();
+                  }
                   trace = push_transaction(trx_meta, fc::time_point::maximum(), fc::microseconds::maximum(),
                                            receipt.cpu_usage_us, true, 0);
                   ++packed_idx;

libraries/chain/include/eosio/chain/exceptions.hpp

@@ -298,6 +298,10 @@ namespace eosio { namespace chain {
                                     3040017, "Transaction includes disallowed extensions (invalid block)" )
       FC_DECLARE_DERIVED_EXCEPTION( tx_resource_exhaustion, transaction_exception,
                                     3040018, "Transaction exceeded transient resource limit" )
+      FC_DECLARE_DERIVED_EXCEPTION( tx_extra_data_error, transaction_exception,
+                                    3040019, "Packed Transaction contains extraneous data" )
+      FC_DECLARE_DERIVED_EXCEPTION( tx_compression_error, transaction_exception,
+                                    3040020, "Packed Transaction compression not allowed" )
 
 
    FC_DECLARE_DERIVED_EXCEPTION( action_validate_exception, chain_exception,

libraries/chain/include/eosio/chain/protocol_feature_manager.hpp

@@ -39,6 +39,7 @@ enum class builtin_protocol_feature_t : uint32_t {
    disable_deferred_trxs_stage_1 = 22,
    disable_deferred_trxs_stage_2 = 23,
    savanna = 24,
+   packed_transaction_restrictions = 25,
    reserved_private_fork_protocol_features = 500000,
 };
 

libraries/chain/include/eosio/chain/transaction.hpp

@@ -158,6 +158,14 @@ namespace eosio { namespace chain {
       packed_transaction( bytes&& packed_txn, vector<signature_type>&& sigs, vector<bytes>&& cfd, compression_type _compression );
       packed_transaction( transaction&& t, vector<signature_type>&& sigs, bytes&& packed_cfd, compression_type _compression );
 
+      // uncompress if compressed, validate no extra data
+      void normalize();
+
+      // throw tx_extra_data_error exception if extra data in packed_transaction
+      void validate_no_extra_data() const;
+      // throw tx_compression_error exception if packed_transaction is not compression_type::none
+      void validate_no_compression() const;
+
       friend bool operator==(const packed_transaction& lhs, const packed_transaction& rhs) {
          return std::tie(lhs.signatures, lhs.compression, lhs.packed_context_free_data, lhs.packed_trx) ==
                 std::tie(rhs.signatures, rhs.compression, rhs.packed_context_free_data, rhs.packed_trx);
@@ -202,6 +210,7 @@ namespace eosio { namespace chain {
       // cache unpacked trx, for thread safety do not modify after construction
       signed_transaction                      unpacked_trx;
       transaction_id_type                     trx_id;
+      bool                                    extra_data = false;
    };
 
    using packed_transaction_ptr = std::shared_ptr<const packed_transaction>;

libraries/chain/protocol_feature_manager.cpp

@@ -363,6 +363,18 @@ host function call will trigger a transition to the Savanna consensus algorithm.
               builtin_protocol_feature_t::disable_deferred_trxs_stage_2
             }
          } )
+         (  builtin_protocol_feature_t::packed_transaction_restrictions, builtin_protocol_feature_spec{
+            "PACKED_TRANSACTION_RESTRICTIONS",
+            fc::variant("b0025cdde6af98ca57393dfd6af8846e67965072a41d1b0d95ede3d59042356c").as<digest_type>(),
+            // SHA256 hash of the raw message below within the comment delimiters (exclude newline after /*) (do not modify message below).
+/*
+Builtin protocol feature: PACKED_TRANSACTION_RESTRICTIONS
+
+Do not allow extra-data in packed_transaction.
+Do not allow on-chain zlib compressed packed_transaction.
+*/
+            {builtin_protocol_feature_t::savanna}
+         } )
    ;
 
 

libraries/chain/transaction.cpp

@@ -198,8 +198,14 @@ static vector<bytes> unpack_context_free_data(const bytes& data) {
    return fc::raw::unpack< vector<bytes> >(data);
 }
 
-static transaction unpack_transaction(const bytes& data) {
-   return fc::raw::unpack<transaction>(data);
+static transaction unpack_transaction(const bytes& data, bool& extra_data) {
+   try  {
+      transaction tmp;
+      fc::datastream<const char*>  ds( data.data(), size_t(data.size()) );
+      fc::raw::unpack(ds,tmp);
+      extra_data = ds.remaining();
+      return tmp;
+   } FC_RETHROW_EXCEPTIONS( warn, "error unpacking transaction" )
 }
 
 static bytes zlib_decompress(const bytes& data) {
@@ -228,9 +234,9 @@ static vector<bytes> zlib_decompress_context_free_data(const bytes& data) {
    return unpack_context_free_data(out);
 }
 
-static transaction zlib_decompress_transaction(const bytes& data) {
+static transaction zlib_decompress_transaction(const bytes& data, bool& extra_data) {
    bytes out = zlib_decompress(data);
-   return unpack_transaction(out);
+   return unpack_transaction(out, extra_data);
 }
 
 static bytes pack_transaction(const transaction& t) {
@@ -305,6 +311,29 @@ packed_transaction::packed_transaction( transaction&& t, vector<signature_type>&
    }
 }
 
+void packed_transaction::normalize() {
+   validate_no_extra_data();
+
+   switch( compression ) {
+      case compression_type::none:
+         return;
+      case compression_type::zlib:
+         break;
+      default:
+         EOS_THROW( unknown_transaction_compression, "Unknown transaction compression algorithm" );
+   }
+   packed_trx = zlib_decompress(packed_trx);
+   compression = compression_type::none;
+}
+
+void packed_transaction::validate_no_extra_data() const {
+   EOS_ASSERT(!extra_data, tx_extra_data_error, "Extra-data not allowed in packed_transaction");
+}
+
+void packed_transaction::validate_no_compression() const {
+   EOS_ASSERT(get_compression() == compression_type::none, tx_compression_error, "Compressed packed_transaction not allowed");
+}
+
 void packed_transaction::reflector_init()
 {
    // called after construction, but always on the same thread and before packed_transaction passed to any other threads
@@ -320,10 +349,10 @@ void packed_transaction::local_unpack_transaction(vector<bytes>&& context_free_d
    try {
       switch( compression ) {
          case compression_type::none:
-            unpacked_trx = signed_transaction( unpack_transaction( packed_trx ), signatures, std::move(context_free_data) );
+            unpacked_trx = signed_transaction( unpack_transaction( packed_trx, extra_data ), signatures, std::move(context_free_data) );
             break;
          case compression_type::zlib:
-            unpacked_trx = signed_transaction( zlib_decompress_transaction( packed_trx ), signatures, std::move(context_free_data) );
+            unpacked_trx = signed_transaction( zlib_decompress_transaction( packed_trx, extra_data ), signatures, std::move(context_free_data) );
             break;
          default:
             EOS_THROW( unknown_transaction_compression, "Unknown transaction compression algorithm" );

plugins/chain_plugin/chain_plugin.cpp

@@ -2119,13 +2119,14 @@ void read_write::push_block(read_write::push_block_params&& params, next_functio
 
 void read_write::push_transaction(const read_write::push_transaction_params& params, next_function<read_write::push_transaction_results> next) {
    try {
-      auto pretty_input = std::make_shared<packed_transaction>();
+      auto ptrx = std::make_shared<packed_transaction>();
       auto resolver = caching_resolver(make_resolver(db, abi_serializer_max_time, throw_on_yield::yes));
       try {
-         abi_serializer::from_variant(params, *pretty_input, resolver, abi_serializer_max_time);
+         abi_serializer::from_variant(params, *ptrx, resolver, abi_serializer_max_time);
       } EOS_RETHROW_EXCEPTIONS(chain::packed_transaction_type_exception, "Invalid packed transaction")
+      ptrx->normalize();
 
-      app().get_method<incoming::methods::transaction_async>()(pretty_input, true, transaction_metadata::trx_type::input, false,
+      app().get_method<incoming::methods::transaction_async>()(ptrx, true, transaction_metadata::trx_type::input, false,
             [this, next](const next_function_variant<transaction_trace_ptr>& result) -> void {
          if (std::holds_alternative<fc::exception_ptr>(result)) {
             next(std::get<fc::exception_ptr>(result));
@@ -2247,6 +2248,7 @@ void api_base::send_transaction_gen(API &api, send_transaction_params_t params,
       try {
          abi_serializer::from_variant(params.transaction, *ptrx, resolver, api.abi_serializer_max_time);
       } EOS_RETHROW_EXCEPTIONS(packed_transaction_type_exception, "Invalid packed transaction")
+      ptrx->normalize();
 
       bool retry = false;
       std::optional<uint16_t> retry_num_blocks;

plugins/net_plugin/net_plugin.cpp

@@ -3229,6 +3229,8 @@ namespace eosio {
       const auto& tid = ptr->id();
       peer_dlog( this, "received packed_transaction ${id}", ("id", tid) );
 
+      ptr->normalize();
+
       if (message_length < def_trx_notice_min_size) {
          // transfer packed transaction is ~170 bytes, transaction notice is 41 bytes
          fc_dlog( logger, "trx notice not sent, trx size ${s}", ("s", message_length));