Merge pull request #5451 from EOSIO/notify-ram-charging-soft-fix

Subjectively fail transaction if contract bills RAM to another account within a notification handler

contracts/test_api/test_action.cpp

@@ -225,3 +225,28 @@ void test_action::test_assert_code() {
    eosio_assert( total == sizeof(uint64_t), "total == sizeof(uint64_t)");
    eosio_assert_code( false, code );
 }
+
+void test_action::test_ram_billing_in_notify(uint64_t receiver, uint64_t code, uint64_t action) {
+   uint128_t tmp = 0;
+   uint32_t total = read_action_data(&tmp, sizeof(uint128_t));
+   eosio_assert( total == sizeof(uint128_t), "total == sizeof(uint128_t)");
+
+   uint64_t to_notify = tmp >> 64;
+   uint64_t payer = tmp & 0xFFFFFFFFFFFFFFFFULL;
+
+   if( code == receiver ) {
+      eosio::require_recipient( to_notify );
+   } else {
+      eosio_assert( to_notify == receiver, "notified recipient other than the one specified in to_notify" );
+
+      // Remove main table row if it already exists.
+      int itr = db_find_i64( receiver, N(notifytest), N(notifytest), N(notifytest) );
+      if( itr >= 0 )
+         db_remove_i64( itr );
+
+      // Create the main table row simply for the purpose of charging code more RAM.
+      if( payer != 0 )
+         db_store_i64(N(notifytest), N(notifytest), payer, N(notifytest), &to_notify, sizeof(to_notify) );
+   }
+
+}

contracts/test_api/test_api.cpp

@@ -79,6 +79,7 @@ extern "C" {
       WASM_TEST_HANDLER_EX(test_action, test_current_receiver);
       WASM_TEST_HANDLER(test_action, test_publication_time);
       WASM_TEST_HANDLER(test_action, test_assert_code);
+      WASM_TEST_HANDLER_EX(test_action, test_ram_billing_in_notify);
 
       // test named actions
       // We enforce action name matches action data type name, so name mangling will not work for these tests.

contracts/test_api/test_api.hpp

@@ -57,7 +57,6 @@ struct test_print {
 };
 
 struct test_action {
-
   static void read_action_normal();
   static void read_action_to_0();
   static void read_action_to_64k();
@@ -73,6 +72,7 @@ struct test_action {
   static void test_current_receiver(uint64_t receiver, uint64_t code, uint64_t action);
   static void test_publication_time();
   static void test_assert_code();
+  static void test_ram_billing_in_notify(uint64_t receiver, uint64_t code, uint64_t action);
 };
 
 struct test_db {

libraries/chain/apply_context.cpp

@@ -301,6 +301,8 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a
          });
    }
 
+   EOS_ASSERT( control.is_ram_billing_in_notify_allowed() || (receiver == act.account) || (receiver == payer) || privileged,
+               subjective_block_production_exception, "Cannot charge RAM to other accounts during notify." );
    trx_context.add_ram_usage( payer, (config::billable_size_v<generated_transaction_object> + trx_size) );
 }
 
@@ -362,6 +364,8 @@ bytes apply_context::get_packed_transaction() {
 void apply_context::update_db_usage( const account_name& payer, int64_t delta ) {
    if( delta > 0 ) {
       if( !(privileged || payer == account_name(receiver)) ) {
+         EOS_ASSERT( control.is_ram_billing_in_notify_allowed() || (receiver == act.account),
+                     subjective_block_production_exception, "Cannot charge RAM to other accounts during notify." );
          require_authorization( payer );
       }
    }

libraries/chain/controller.cpp

@@ -1776,6 +1776,10 @@ bool controller::is_producing_block()const {
    return (my->pending->_block_status == block_status::incomplete);
 }
 
+bool controller::is_ram_billing_in_notify_allowed()const {
+   return !is_producing_block() || my->conf.allow_ram_billing_in_notify;
+}
+
 void controller::validate_referenced_accounts( const transaction& trx )const {
    for( const auto& a : trx.context_free_actions ) {
       auto* code = my->db.find<account_object, by_name>(a.account);

libraries/chain/include/eosio/chain/controller.hpp

@@ -65,6 +65,7 @@ namespace eosio { namespace chain {
             bool                     force_all_checks       =  false;
             bool                     disable_replay_opts    =  false;
             bool                     contracts_console      =  false;
+            bool                     allow_ram_billing_in_notify = false;
 
             genesis_state            genesis;
             wasm_interface::vm_type  wasm_runtime = chain::config::default_wasm_runtime;
@@ -204,6 +205,8 @@ namespace eosio { namespace chain {
          void check_key_list( const public_key_type& key )const;
          bool is_producing_block()const;
 
+         bool is_ram_billing_in_notify_allowed()const;
+
          void add_resource_greylist(const account_name &name);
          void remove_resource_greylist(const account_name &name);
          bool is_resource_greylisted(const account_name &name) const;

plugins/chain_plugin/chain_plugin.cpp

@@ -244,6 +244,8 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip
           "Chain validation mode (\"full\" or \"light\").\n"
           "In \"full\" mode all incoming blocks will be fully validated.\n"
           "In \"light\" mode all incoming blocks headers will be fully validated; transactions in those validated blocks will be trusted \n")
+         ("disable-ram-billing-notify-checks", bpo::bool_switch()->default_value(false),
+          "Disable the check which subjectively fails a transaction if a contract bills more RAM to another account within the context of a notification handler (i.e. when the receiver is not the code of the action).")
          ;
 
 // TODO: rate limiting
@@ -405,6 +407,7 @@ void chain_plugin::plugin_initialize(const variables_map& options) {
       my->chain_config->force_all_checks = options.at( "force-all-checks" ).as<bool>();
       my->chain_config->disable_replay_opts = options.at( "disable-replay-opts" ).as<bool>();
       my->chain_config->contracts_console = options.at( "contracts-console" ).as<bool>();
+      my->chain_config->allow_ram_billing_in_notify = options.at( "disable-ram-billing-notify-checks" ).as<bool>();
 
       if( options.count( "extract-genesis-json" ) || options.at( "print-genesis-json" ).as<bool>()) {
          genesis_state gs;

unittests/api_tests.cpp

@@ -437,6 +437,27 @@ BOOST_FIXTURE_TEST_CASE(action_tests, TESTER) { try {
    BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW() }
 
+BOOST_FIXTURE_TEST_CASE(ram_billing_in_notify_tests, TESTER) { try {
+   produce_blocks(2);
+   create_account( N(testapi) );
+   create_account( N(testapi2) );
+   produce_blocks(10);
+   set_code( N(testapi), test_api_wast );
+   produce_blocks(1);
+   set_code( N(testapi2), test_api_wast );
+   produce_blocks(1);
+
+   BOOST_CHECK_EXCEPTION( CALL_TEST_FUNCTION( *this, "test_action", "test_ram_billing_in_notify", fc::raw::pack( ((unsigned __int128)N(testapi2) << 64) | N(testapi) ) ),
+                          subjective_block_production_exception, fc_exception_message_is("Cannot charge RAM to other accounts during notify.") );
+
+
+   CALL_TEST_FUNCTION( *this, "test_action", "test_ram_billing_in_notify", fc::raw::pack( ((unsigned __int128)N(testapi2) << 64) | 0 ) );
+
+   CALL_TEST_FUNCTION( *this, "test_action", "test_ram_billing_in_notify", fc::raw::pack( ((unsigned __int128)N(testapi2) << 64) | N(testapi2) ) );
+
+   BOOST_REQUIRE_EQUAL( validate(), true );
+} FC_LOG_AND_RETHROW() }
+
 /*************************************************************************************
  * context free action tests
  *************************************************************************************/