From 468843a286faec3a3df9f6dcc50f93a4f7efea3c Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 7 Dec 2020 07:22:11 -0600 Subject: [PATCH] Consolidated Security Fixes for 2.0.8 - Adjust `eos-vm-oc` string intrinsic to perform as intended. - Adjust CPU validation logic for unapplied transactions. Co-Authored-By: Kevin Heifner Co-Authored-By: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> --- .../include/eosio/chain/webassembly/eos-vm-oc.hpp | 2 +- libraries/chain/transaction_context.cpp | 6 ++++-- plugins/producer_plugin/producer_plugin.cpp | 15 +++++++++++++-- unittests/wasm_tests.cpp | 1 + 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc.hpp index bf672c35eb5..10e73d1b1ee 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc.hpp @@ -88,7 +88,7 @@ inline char* null_terminated_ptr_impl(uint64_t ptr) "mov %[Ptr],%[Scratch]\n" "1:\n" //start loop looking for either 0, or until we SEGV "inc %[Scratch]\n" - "cmpb $0,%%gs:(%[Scratch])\n" + "cmpb $0,%%gs:-1(%[Scratch])\n" "jne 1b\n" "2:\n" "add %%gs:%c[linearMemoryStart], %[Ptr]\n" //add address of linear memory 0 to ptr diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index 523fbc6a17a..136282ceb40 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -167,8 +167,10 @@ namespace eosio { namespace chain { } if( !explicit_billed_cpu_time ) { - // if account no longer has enough cpu to exec trx, don't try - validate_account_cpu_usage( billed_cpu_time_us, account_cpu_limit, true ); + // Fail early if amount of the previous speculative execution is within 10% of remaining account cpu available + int64_t validate_account_cpu_limit = account_cpu_limit - EOS_PERCENT( account_cpu_limit, 10 * config::percent_1 ); + if( validate_account_cpu_limit < 0 ) validate_account_cpu_limit = 0; + validate_account_cpu_usage( billed_cpu_time_us, validate_account_cpu_limit, true ); } eager_net_limit = (eager_net_limit/8)*8; // Round down to nearest multiple of word size (8 bytes) so check_net_usage can be efficient diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index bb57e6b9429..f856c978468 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1706,7 +1706,15 @@ bool producer_plugin_impl::process_unapplied_trxs( const fc::time_point& deadlin const transaction_metadata_ptr trx = itr->trx_meta; ++num_processed; try { - auto trx_deadline = fc::time_point::now() + fc::milliseconds( _max_transaction_time_ms ); + auto start = fc::time_point::now(); + auto trx_deadline = start + fc::milliseconds( _max_transaction_time_ms ); + + auto prev_billed_cpu_time_us = trx->billed_cpu_time_us; + if( prev_billed_cpu_time_us > 0 ) { + auto prev_billed_plus50 = prev_billed_cpu_time_us + EOS_PERCENT( prev_billed_cpu_time_us, 50 * config::percent_1 ); + auto trx_dl = start + fc::microseconds( prev_billed_plus50 ); + if( trx_dl < trx_deadline ) trx_deadline = trx_dl; + } bool deadline_is_subjective = false; if( _max_transaction_time_ms < 0 || (_pending_block_mode == pending_block_mode::producing && deadline < trx_deadline) ) { @@ -1714,7 +1722,7 @@ bool producer_plugin_impl::process_unapplied_trxs( const fc::time_point& deadlin trx_deadline = deadline; } - auto trace = chain.push_transaction( trx, trx_deadline, trx->billed_cpu_time_us, false ); + auto trace = chain.push_transaction( trx, trx_deadline, prev_billed_cpu_time_us, false ); if( trace->except ) { if( exception_is_exhausted( *trace->except, deadline_is_subjective ) ) { if( block_is_exhausted() ) { @@ -1725,6 +1733,9 @@ bool producer_plugin_impl::process_unapplied_trxs( const fc::time_point& deadlin // don't erase, subjective failure so try again next time } else { // this failed our configured maximum transaction time, we don't want to replay it + fc_dlog( _log, "Failed ${c} trx, prev billed: ${p}us, ran: ${r}us, id: ${id}", + ("c", trace->except->code())("p", prev_billed_cpu_time_us) + ("r", fc::time_point::now() - start)("id", trx->id()) ); ++num_failed; if( itr->next ) itr->next( trace ); itr = _unapplied_transactions.erase( itr ); diff --git a/unittests/wasm_tests.cpp b/unittests/wasm_tests.cpp index e4a64a7f6e4..453b059239d 100644 --- a/unittests/wasm_tests.cpp +++ b/unittests/wasm_tests.cpp @@ -1990,6 +1990,7 @@ BOOST_AUTO_TEST_CASE( billed_cpu_test ) try { chain.produce_block( fc::days(1) ); // produce for one day to reset account cpu cpu_limit = mgr.get_account_cpu_limit_ex(acc).first.max; + cpu_limit -= EOS_PERCENT( cpu_limit, 10 * config::percent_1 ); // transaction_context verifies within 10%, so subtract 10% out ptrx = create_trx(0); BOOST_CHECK_LT( cpu_limit+1, max_cpu_time_us ); // needs to be less or this just tests the same thing as max_cpu_time_us test above