Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

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

Merged
merged 3 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions contracts/test_api/test_action.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) );
}

}
1 change: 1 addition & 0 deletions contracts/test_api/test_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion contracts/test_api/test_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/apply_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be moved to trx_context.add_ram_usage so that all cases are covered in one spot. This seems to miss eosio_contract.cpp apply_eosio_* call and transaction_context.schedule_transaction.

Copy link
Contributor Author

@arhag arhag Aug 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The apply_eosio_* calls are not a problem since they never run in the context of a notification handler. Similarly with transaction_context::schedule_transaction which does not even run in the context of executing an action. The payer in that latter case is the first authorizer of the transaction which needs to be allowed to be billed for the RAM of the delayed transaction.

I didn't put the check in transaction_context::add_ram_usage because it doesn't have access to the apply_context fields, such as receiver and act.account, needed to make the decision of whether or not to throw the subjective_block_production_exception.

}

Expand Down Expand Up @@ -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 );
}
}
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 21 additions & 0 deletions unittests/api_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
*************************************************************************************/
Expand Down