From 8425270150e0026279532cf1d6eed0768f959739 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Tue, 15 Aug 2017 12:13:36 -0500 Subject: [PATCH 1/2] Fix tracking of used_authorizations across notified accounts Previously, only the used_authorizations from the original message were being tracked, even though the original message might notify other accounts which in turn might require more authorizations. This is now fixed so that all required auths from all handlers called in the processing of a message and its notifications are tracked. This fixes "irrelevant authority" errors which were being thrown incorrectly. --- libraries/chain/chain_controller.cpp | 43 +++++++------------ .../include/eos/chain/chain_controller.hpp | 3 +- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 25b0d6daa41..c183be56768 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -607,43 +607,18 @@ void chain_controller::validate_expiration(const SignedTransaction& trx) const } FC_CAPTURE_AND_RETHROW((trx)) } -/** - * When processing a message it needs to run: - * - * code::precondition( message.code, message.apply ) - * code::apply( message.code, message.apply ) - * - * Then for each recipient it needs to run - * - * ```` - * recipient::precondition( message.code, message.apply ) - * recipient::apply( message.code, message.apply ) - * ``` - * - * The data that can be read is anything declared in trx.scope - * The data that can be written is anything stored in * / [code | recipient] / * - * - * The order of execution of precondition and apply can impact the validity of the - * entire message. - */ void chain_controller::process_message(const ProcessedTransaction& trx, AccountName code, - const Message& message, MessageOutput& output) { + const Message& message, MessageOutput& output, apply_context* parent_context) { apply_context apply_ctx(*this, _db, trx, message, code); apply_message(apply_ctx); - // process_message recurses for each notified account, but we only want to run this check at the top level - if (code == message.code && (_skip_flags & skip_authority_check) == false) - EOS_ASSERT(apply_ctx.all_authorizations_used(), tx_irrelevant_auth, - "Message declared authorities it did not need: ${unused}", - ("unused", apply_ctx.unused_authorizations())("message", message)); - output.notify.reserve( apply_ctx.notified.size() ); for( uint32_t i = 0; i < apply_ctx.notified.size(); ++i ) { try { auto notify_code = apply_ctx.notified[i]; output.notify.push_back( {notify_code} ); - process_message( trx, notify_code, message, output.notify.back().output ); + process_message(trx, notify_code, message, output.notify.back().output, &apply_ctx); } FC_CAPTURE_AND_RETHROW((apply_ctx.notified[i])) } @@ -654,8 +629,20 @@ void chain_controller::process_message(const ProcessedTransaction& trx, AccountN } for( auto& asynctrx : apply_ctx.async_transactions ) { - output.async_transactions.emplace_back( std::move( asynctrx ) ); + output.async_transactions.emplace_back( std::move( asynctrx ) ); } + + // propagate used_authorizations up the context chain + if (parent_context != nullptr) + for (int i = 0; i < apply_ctx.used_authorizations.size(); ++i) + if (apply_ctx.used_authorizations[i]) + parent_context->used_authorizations[i] = true; + + // process_message recurses for each notified account, but we only want to run this check at the top level + if (parent_context == nullptr && (_skip_flags & skip_authority_check) == false) + EOS_ASSERT(apply_ctx.all_authorizations_used(), tx_irrelevant_auth, + "Message declared authorities it did not need: ${unused}", + ("unused", apply_ctx.unused_authorizations())("message", message)); } void chain_controller::apply_message(apply_context& context) diff --git a/libraries/chain/include/eos/chain/chain_controller.hpp b/libraries/chain/include/eos/chain/chain_controller.hpp index d56df89baa8..202cee399e5 100644 --- a/libraries/chain/include/eos/chain/chain_controller.hpp +++ b/libraries/chain/include/eos/chain/chain_controller.hpp @@ -302,7 +302,8 @@ namespace eos { namespace chain { types::AccountName code_account, types::FuncName type) const; - void process_message(const ProcessedTransaction& trx, AccountName code, const Message& message, MessageOutput& output); + void process_message(const ProcessedTransaction& trx, AccountName code, const Message& message, + MessageOutput& output, apply_context* parent_context = nullptr); void apply_message(apply_context& c); bool should_check_for_duplicate_transactions()const { return !(_skip_flags&skip_transaction_dupe_check); } From 5a60917c5352be06766e8cc5b70252624b218c60 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Tue, 15 Aug 2017 12:18:32 -0500 Subject: [PATCH 2/2] Add missing require_authorization eos::setcode was not requiring authorization from the account whose code was being set. This is definitely incorrect. Alice should not be able to set Bob's code without Bob's authorization. :P --- libraries/native_contract/eos_contract.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/native_contract/eos_contract.cpp b/libraries/native_contract/eos_contract.cpp index 8a39ff9cb27..4c5d337bd91 100644 --- a/libraries/native_contract/eos_contract.cpp +++ b/libraries/native_contract/eos_contract.cpp @@ -195,6 +195,8 @@ void apply_eos_setcode(apply_context& context) { auto& db = context.mutable_db; auto msg = context.msg.as(); + context.require_authorization(msg.account); + FC_ASSERT( msg.vmtype == 0 ); FC_ASSERT( msg.vmversion == 0 );