From 85cf4d08f6fd3b7c79a772f20b7b7c6c3166e41e Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Thu, 13 Dec 2018 14:43:07 -0500 Subject: [PATCH 1/2] bump version to 1.5.1 --- CMakeLists.txt | 2 +- Docker/README.md | 4 ++-- README.md | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c711ae90db9..a1be6595ae3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,7 +35,7 @@ set( CXX_STANDARD_REQUIRED ON) set(VERSION_MAJOR 1) set(VERSION_MINOR 5) -set(VERSION_PATCH 0) +set(VERSION_PATCH 1) if(VERSION_SUFFIX) set(VERSION_FULL "${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_PATCH}-${VERSION_SUFFIX}") diff --git a/Docker/README.md b/Docker/README.md index 6647d61726f..bfb20bb08d6 100644 --- a/Docker/README.md +++ b/Docker/README.md @@ -20,10 +20,10 @@ cd eos/Docker docker build . -t eosio/eos ``` -The above will build off the most recent commit to the master branch by default. If you would like to target a specific branch/tag, you may use a build argument. For example, if you wished to generate a docker image based off of the v1.5.0 tag, you could do the following: +The above will build off the most recent commit to the master branch by default. If you would like to target a specific branch/tag, you may use a build argument. For example, if you wished to generate a docker image based off of the v1.5.1 tag, you could do the following: ```bash -docker build -t eosio/eos:v1.5.0 --build-arg branch=v1.5.0 . +docker build -t eosio/eos:v1.5.1 --build-arg branch=v1.5.1 . ``` By default, the symbol in eosio.system is set to SYS. You can override this using the symbol argument while building the docker image. diff --git a/README.md b/README.md index 9dbb16dc1c9..5eb8be73290 100644 --- a/README.md +++ b/README.md @@ -39,13 +39,13 @@ $ brew remove eosio ``` #### Ubuntu 18.04 Debian Package Install ```sh -$ wget https://github.com/eosio/eos/releases/download/v1.5.0/eosio_1.5.0-1-ubuntu-18.04_amd64.deb -$ sudo apt install ./eosio_1.5.0-1-ubuntu-18.04_amd64.deb +$ wget https://github.com/eosio/eos/releases/download/v1.5.1/eosio_1.5.1-1-ubuntu-18.04_amd64.deb +$ sudo apt install ./eosio_1.5.1-1-ubuntu-18.04_amd64.deb ``` #### Ubuntu 16.04 Debian Package Install ```sh -$ wget https://github.com/eosio/eos/releases/download/v1.5.0/eosio_1.5.0-1-ubuntu-16.04_amd64.deb -$ sudo apt install ./eosio_1.5.0-1-ubuntu-16.04_amd64.deb +$ wget https://github.com/eosio/eos/releases/download/v1.5.1/eosio_1.5.1-1-ubuntu-16.04_amd64.deb +$ sudo apt install ./eosio_1.5.1-1-ubuntu-16.04_amd64.deb ``` #### Debian Package Uninstall ```sh @@ -53,8 +53,8 @@ $ sudo apt remove eosio ``` #### Centos RPM Package Install ```sh -$ wget https://github.com/eosio/eos/releases/download/v1.5.0/eosio-1.5.0-1.el7.x86_64.rpm -$ sudo yum install ./eosio-1.5.0-1.el7.x86_64.rpm +$ wget https://github.com/eosio/eos/releases/download/v1.5.1/eosio-1.5.1-1.el7.x86_64.rpm +$ sudo yum install ./eosio-1.5.1-1.el7.x86_64.rpm ``` #### Centos RPM Package Uninstall ```sh @@ -62,8 +62,8 @@ $ sudo yum remove eosio.cdt ``` #### Fedora RPM Package Install ```sh -$ wget https://github.com/eosio/eos/releases/download/v1.5.0/eosio-1.5.0-1.fc27.x86_64.rpm -$ sudo yum install ./eosio-1.5.0-1.fc27.x86_64.rpm +$ wget https://github.com/eosio/eos/releases/download/v1.5.1/eosio-1.5.1-1.fc27.x86_64.rpm +$ sudo yum install ./eosio-1.5.1-1.fc27.x86_64.rpm ``` #### Fedora RPM Package Uninstall ```sh From 5f2a0c59a5155adbea4885eedc10b49023490214 Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Thu, 13 Dec 2018 14:30:13 -0500 Subject: [PATCH 2/2] Consolidated Security Fixes for 1.5.1 - Only allow authorizations that are satisfiable by `eosio.code` for self-addressed deferred transactions - Only allow authorizations that are satisfiable by `eosio.code` OR on the parent action for self-addressed inline actions sent from direct actions - Only allow authorizations that are satisfiable by `eosio.code` for self-addressed inline actions sent from recipient handlers Co-authored-by: arhag Co-authored-by: Bart Wyatt --- libraries/chain/apply_context.cpp | 108 ++++++++++++---- libraries/chain/authorization_manager.cpp | 11 +- .../eosio/chain/authorization_manager.hpp | 3 +- unittests/api_tests.cpp | 115 +++++++++--------- unittests/whitelist_blacklist_tests.cpp | 2 +- 5 files changed, 155 insertions(+), 84 deletions(-) diff --git a/libraries/chain/apply_context.cpp b/libraries/chain/apply_context.cpp index c7b0fc37125..bedf5b4d616 100644 --- a/libraries/chain/apply_context.cpp +++ b/libraries/chain/apply_context.cpp @@ -208,6 +208,15 @@ void apply_context::execute_inline( action&& a ) { bool enforce_actor_whitelist_blacklist = trx_context.enforce_whiteblacklist && control.is_producing_block(); flat_set actors; + bool disallow_send_to_self_bypass = false; // eventually set to whether the appropriate protocol feature has been activated + bool send_to_self = (a.account == receiver); + bool inherit_parent_authorizations = (!disallow_send_to_self_bypass && send_to_self && (receiver == act.account) && control.is_producing_block()); + + flat_set inherited_authorizations; + if( inherit_parent_authorizations ) { + inherited_authorizations.reserve( a.authorization.size() ); + } + for( const auto& auth : a.authorization ) { auto* actor = control.db().find(auth.actor); EOS_ASSERT( actor != nullptr, action_validate_exception, @@ -217,26 +226,49 @@ void apply_context::execute_inline( action&& a ) { ("permission", auth) ); if( enforce_actor_whitelist_blacklist ) actors.insert( auth.actor ); + + if( inherit_parent_authorizations && std::find(act.authorization.begin(), act.authorization.end(), auth) != act.authorization.end() ) { + inherited_authorizations.insert( auth ); + } } if( enforce_actor_whitelist_blacklist ) { control.check_actor_list( actors ); } - // No need to check authorization if: replaying irreversible blocks; contract is privileged; or, contract is calling itself. - if( !control.skip_auth_check() && !privileged && a.account != receiver ) { - control.get_authorization_manager() - .check_authorization( {a}, - {}, - {{receiver, config::eosio_code_name}}, - control.pending_block_time() - trx_context.published, - std::bind(&transaction_context::checktime, &this->trx_context), - false - ); - - //QUESTION: Is it smart to allow a deferred transaction that has been delayed for some time to get away - // with sending an inline action that requires a delay even though the decision to send that inline - // action was made at the moment the deferred transaction was executed with potentially no forewarning? + // No need to check authorization if replaying irreversible blocks or contract is privileged + if( !control.skip_auth_check() && !privileged ) { + try { + control.get_authorization_manager() + .check_authorization( {a}, + {}, + {{receiver, config::eosio_code_name}}, + control.pending_block_time() - trx_context.published, + std::bind(&transaction_context::checktime, &this->trx_context), + false, + inherited_authorizations + ); + + //QUESTION: Is it smart to allow a deferred transaction that has been delayed for some time to get away + // with sending an inline action that requires a delay even though the decision to send that inline + // action was made at the moment the deferred transaction was executed with potentially no forewarning? + } catch( const fc::exception& e ) { + if( disallow_send_to_self_bypass || !send_to_self ) { + throw; + } else if( control.is_producing_block() ) { + subjective_block_production_exception new_exception(FC_LOG_MESSAGE( error, "Authorization failure with inline action sent to self")); + for (const auto& log: e.get_log()) { + new_exception.append_log(log); + } + throw new_exception; + } + } catch( ... ) { + if( disallow_send_to_self_bypass || !send_to_self ) { + throw; + } else if( control.is_producing_block() ) { + EOS_THROW(subjective_block_production_exception, "Unexpected exception occurred validating inline action sent to self"); + } + } } _inline_actions.emplace_back( move(a) ); @@ -276,16 +308,30 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a require_authorization(payer); /// uses payer's storage } - // if a contract is deferring only actions to itself then there is no need - // to check permissions, it could have done everything anyway. - bool check_auth = false; - for( const auto& act : trx.actions ) { - if( act.account != receiver ) { - check_auth = true; - break; + // Originally this code bypassed authorization checks if a contract was deferring only actions to itself. + // The idea was that the code could already do whatever the deferred transaction could do, so there was no point in checking authorizations. + // But this is not true. The original implementation didn't validate the authorizations on the actions which allowed for privilege escalation. + // It would make it possible to bill RAM to some unrelated account. + // Furthermore, even if the authorizations were forced to be a subset of the current action's authorizations, it would still violate the expectations + // of the signers of the original transaction, because the deferred transaction would allow billing more CPU and network bandwidth than the maximum limit + // specified on the original transaction. + // So, the deferred transaction must always go through the authorization checking if it is not sent by a privileged contract. + // However, the old logic must still be considered because it cannot objectively change until a consensus protocol upgrade. + + bool disallow_send_to_self_bypass = false; // eventually set to whether the appropriate protocol feature has been activated + + auto is_sending_only_to_self = [&trx]( const account_name& self ) { + bool send_to_self = true; + for( const auto& act : trx.actions ) { + if( act.account != self ) { + send_to_self = false; + break; + } } - } - if( check_auth ) { + return send_to_self; + }; + + try { control.get_authorization_manager() .check_authorization( trx.actions, {}, @@ -294,6 +340,22 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a std::bind(&transaction_context::checktime, &this->trx_context), false ); + } catch( const fc::exception& e ) { + if( disallow_send_to_self_bypass || !is_sending_only_to_self(receiver) ) { + throw; + } else if( control.is_producing_block() ) { + subjective_block_production_exception new_exception(FC_LOG_MESSAGE( error, "Authorization failure with sent deferred transaction consisting only of actions to self")); + for (const auto& log: e.get_log()) { + new_exception.append_log(log); + } + throw new_exception; + } + } catch( ... ) { + if( disallow_send_to_self_bypass || !is_sending_only_to_self(receiver) ) { + throw; + } else if( control.is_producing_block() ) { + EOS_THROW(subjective_block_production_exception, "Unexpected exception occurred validating sent deferred transaction consisting only of actions to self"); + } } } diff --git a/libraries/chain/authorization_manager.cpp b/libraries/chain/authorization_manager.cpp index 832f69c71cd..6725468cf97 100644 --- a/libraries/chain/authorization_manager.cpp +++ b/libraries/chain/authorization_manager.cpp @@ -431,7 +431,8 @@ namespace eosio { namespace chain { const flat_set& provided_permissions, fc::microseconds provided_delay, const std::function& _checktime, - bool allow_unused_keys + bool allow_unused_keys, + const flat_set& satisfied_authorizations )const { const auto& checktime = ( static_cast(_checktime) ? _checktime : _noop_checktime ); @@ -488,9 +489,11 @@ namespace eosio { namespace chain { } } - auto res = permissions_to_satisfy.emplace( declared_auth, delay ); - if( !res.second && res.first->second > delay) { // if the declared_auth was already in the map and with a higher delay - res.first->second = delay; + if( satisfied_authorizations.find( declared_auth ) == satisfied_authorizations.end() ) { + auto res = permissions_to_satisfy.emplace( declared_auth, delay ); + if( !res.second && res.first->second > delay) { // if the declared_auth was already in the map and with a higher delay + res.first->second = delay; + } } } } diff --git a/libraries/chain/include/eosio/chain/authorization_manager.hpp b/libraries/chain/include/eosio/chain/authorization_manager.hpp index 9a75b5f80b1..a6df7ad2568 100644 --- a/libraries/chain/include/eosio/chain/authorization_manager.hpp +++ b/libraries/chain/include/eosio/chain/authorization_manager.hpp @@ -84,7 +84,8 @@ namespace eosio { namespace chain { const flat_set& provided_permissions = flat_set(), fc::microseconds provided_delay = fc::microseconds(0), const std::function& checktime = std::function(), - bool allow_unused_keys = false + bool allow_unused_keys = false, + const flat_set& satisfied_authorizations = flat_set() )const; diff --git a/unittests/api_tests.cpp b/unittests/api_tests.cpp index 60bde1edd99..e889790ba33 100644 --- a/unittests/api_tests.cpp +++ b/unittests/api_tests.cpp @@ -1164,61 +1164,66 @@ BOOST_FIXTURE_TEST_CASE(deferred_transaction_tests, TESTER) { try { produce_blocks(10); -{ - // Trigger a tx which in turn sends a deferred tx with payer != receiver - // Payer is alice in this case, this tx should fail since we don't have the authorization of alice - dtt_action dtt_act1; - dtt_act1.payer = N(alice); - BOOST_CHECK_THROW(CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act1)), missing_auth_exception); - - // Send a tx which in turn sends a deferred tx with the deferred tx's receiver != this tx receiver - // This will include the authorization of the receiver, and impose any related delay associated with the authority - // We set the authorization delay to be 10 sec here, and since the deferred tx delay is set to be 5 sec, so this tx should fail - dtt_action dtt_act2; - dtt_act2.deferred_account = N(testapi2); - dtt_act2.permission_name = N(additional); - dtt_act2.delay_sec = 5; - - auto auth = authority(get_public_key("testapi", name(dtt_act2.permission_name).to_string()), 10); - auth.accounts.push_back( permission_level_weight{{N(testapi), config::eosio_code_name}, 1} ); - - push_action(config::system_account_name, updateauth::get_name(), "testapi", fc::mutable_variant_object() - ("account", "testapi") - ("permission", name(dtt_act2.permission_name)) - ("parent", "active") - ("auth", auth) - ); - push_action(config::system_account_name, linkauth::get_name(), "testapi", fc::mutable_variant_object() - ("account", "testapi") - ("code", name(dtt_act2.deferred_account)) - ("type", name(dtt_act2.deferred_action)) - ("requirement", name(dtt_act2.permission_name))); - BOOST_CHECK_THROW(CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)), unsatisfied_authorization); - - // But if the deferred transaction has a sufficient delay, then it should work. - dtt_act2.delay_sec = 10; - CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)); - - // Meanwhile, if the deferred tx receiver == this tx receiver, the delay will be ignored, this tx should succeed - dtt_action dtt_act3; - dtt_act3.deferred_account = N(testapi); - dtt_act3.permission_name = N(additional); - push_action(config::system_account_name, linkauth::get_name(), "testapi", fc::mutable_variant_object() - ("account", "testapi") - ("code", name(dtt_act3.deferred_account)) - ("type", name(dtt_act3.deferred_action)) - ("requirement", name(dtt_act3.permission_name))); - CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act3)); //will replace existing transaction - - // If we make testapi account to be priviledged account: - // - the deferred transaction will work no matter who is the payer - // - the deferred transaction will not care about the delay of the authorization - push_action(config::system_account_name, N(setpriv), config::system_account_name, mutable_variant_object() - ("account", "testapi") - ("is_priv", 1)); - CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act1)); - CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)); -} + { + // Trigger a tx which in turn sends a deferred tx with payer != receiver + // Payer is alice in this case, this tx should fail since we don't have the authorization of alice + dtt_action dtt_act1; + dtt_act1.payer = N(alice); + BOOST_CHECK_THROW(CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act1)), missing_auth_exception); + + // Send a tx which in turn sends a deferred tx with the deferred tx's receiver != this tx receiver + // This will include the authorization of the receiver, and impose any related delay associated with the authority + // We set the authorization delay to be 10 sec here, and since the deferred tx delay is set to be 5 sec, so this tx should fail + dtt_action dtt_act2; + dtt_act2.deferred_account = N(testapi2); + dtt_act2.permission_name = N(additional); + dtt_act2.delay_sec = 5; + + auto auth = authority(get_public_key("testapi", name(dtt_act2.permission_name).to_string()), 10); + auth.accounts.push_back( permission_level_weight{{N(testapi), config::eosio_code_name}, 1} ); + + push_action(config::system_account_name, updateauth::get_name(), "testapi", fc::mutable_variant_object() + ("account", "testapi") + ("permission", name(dtt_act2.permission_name)) + ("parent", "active") + ("auth", auth) + ); + push_action(config::system_account_name, linkauth::get_name(), "testapi", fc::mutable_variant_object() + ("account", "testapi") + ("code", name(dtt_act2.deferred_account)) + ("type", name(dtt_act2.deferred_action)) + ("requirement", name(dtt_act2.permission_name))); + BOOST_CHECK_THROW(CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)), unsatisfied_authorization); + + // But if the deferred transaction has a sufficient delay, then it should work. + dtt_act2.delay_sec = 10; + CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)); + + // If the deferred tx receiver == this tx receiver, the authorization checking would originally be bypassed. + // But not anymore. Now it should subjectively fail because testapi@additional permission is not unilaterally satisfied by testapi@eosio.code. + dtt_action dtt_act3; + dtt_act3.deferred_account = N(testapi); + dtt_act3.permission_name = N(additional); + push_action(config::system_account_name, linkauth::get_name(), "testapi", fc::mutable_variant_object() + ("account", "testapi") + ("code", name(dtt_act3.deferred_account)) + ("type", name(dtt_act3.deferred_action)) + ("requirement", name(dtt_act3.permission_name))); + BOOST_CHECK_THROW(CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act3)), subjective_block_production_exception); + + // But it should again work if the deferred transaction has a sufficient delay. + dtt_act3.delay_sec = 10; + CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act3)); + + // If we make testapi account to be priviledged account: + // - the deferred transaction will work no matter who is the payer + // - the deferred transaction will not care about the delay of the authorization + push_action(config::system_account_name, N(setpriv), config::system_account_name, mutable_variant_object() + ("account", "testapi") + ("is_priv", 1)); + CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act1)); + CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_tx_with_dtt_action", fc::raw::pack(dtt_act2)); + } BOOST_REQUIRE_EQUAL( validate(), true ); } FC_LOG_AND_RETHROW() } diff --git a/unittests/whitelist_blacklist_tests.cpp b/unittests/whitelist_blacklist_tests.cpp index a7436340330..54b011d0e3d 100644 --- a/unittests/whitelist_blacklist_tests.cpp +++ b/unittests/whitelist_blacklist_tests.cpp @@ -463,7 +463,7 @@ BOOST_AUTO_TEST_CASE( actor_blacklist_inline_deferred ) { try { ); auth = authority(eosio::testing::base_tester::get_public_key("bob", "active")); - auth.accounts.push_back( permission_level_weight{{N(alice), config::active_name}, 1} ); + auth.accounts.push_back( permission_level_weight{{N(alice), config::eosio_code_name}, 1} ); auth.accounts.push_back( permission_level_weight{{N(bob), config::eosio_code_name}, 1} ); tester1.chain->push_action( N(eosio), N(updateauth), N(bob), mvo()