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

Commit

Permalink
Merge pull request #10280 from EOSIO/release-2.1.x-security-patch-358
Browse files Browse the repository at this point in the history 
Fix issue with account query db for release/2.1.x
  • Loading branch information
@nickjjzhao
nickjjzhao authored Apr 27, 2021
2 parents 150614a + 5690dec commit 52ae351
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 55 deletions.
81 changes: 62 additions & 19 deletions plugins/chain_plugin/account_query_db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ using namespace boost::bimaps;
namespace {
/**
* Structure to hold indirect reference to a `property_object` via {owner,name} as well as a non-standard
* index over `last_updated` for roll-back support
* index over `last_updated_height` (which is truncated at the LIB during initialization) for roll-back support
*/
struct permission_info {
// indexed data
chain::name owner;
chain::name name;
fc::time_point last_updated;
uint32_t last_updated_height;

// un-indexed data
uint32_t threshold;
Expand All @@ -38,10 +38,10 @@ namespace {
};

struct by_owner_name;
struct by_last_updated;
struct by_last_updated_height;

/**
* Multi-index providing fast lookup for {owner,name} as well as {last_updated}
* Multi-index providing fast lookup for {owner,name} as well as {last_updated_height}
*/
using permission_info_index_t = multi_index_container<
permission_info,
Expand All @@ -54,8 +54,8 @@ namespace {
>
>,
ordered_non_unique<
tag<by_last_updated>,
member<permission_info, fc::time_point, &permission_info::last_updated>
tag<by_last_updated_height>,
member<permission_info, uint32_t, &permission_info::last_updated_height>
>
>
>;
Expand Down Expand Up @@ -144,8 +144,19 @@ namespace eosio::chain_apis {
auto start = fc::time_point::now();
const auto& index = controller.db().get_index<chain::permission_index>().indices().get<by_id>();

// build a initial time to block number map
const auto lib_num = controller.last_irreversible_block_num();
const auto head_num = controller.head_block_num();

for (uint32_t block_num = lib_num + 1; block_num <= head_num; block_num++) {
const auto block_p = controller.fetch_block_by_number(block_num);
EOS_ASSERT(block_p, chain::plugin_exception, "cannot fetch reversible block ${block_num}, required for account_db initialization", ("block_num", block_num));
time_to_block_num.emplace(block_p->timestamp.to_time_point(), block_num);
}

for (const auto& po : index ) {
const auto& pi = permission_info_index.emplace( permission_info{ po.owner, po.name, po.last_updated, po.auth.threshold } ).first;
uint32_t last_updated_height = last_updated_time_to_height(po.last_updated);
const auto& pi = permission_info_index.emplace( permission_info{ po.owner, po.name, last_updated_height, po.auth.threshold } ).first;
add_to_bimaps(*pi, po);
}
auto duration = fc::time_point::now() - start;
Expand Down Expand Up @@ -186,42 +197,62 @@ namespace eosio::chain_apis {

bool is_rollback_required( const chain::block_state_ptr& bsp ) const {
std::shared_lock read_lock(rw_mutex);
const auto t = bsp->block->timestamp.to_time_point();
const auto& index = permission_info_index.get<by_last_updated>();
const auto bnum = bsp->block->block_num();
const auto& index = permission_info_index.get<by_last_updated_height>();

if (index.empty()) {
return false;
} else {
const auto& pi = (*index.rbegin());
if (pi.last_updated < t) {
if (pi.last_updated_height < bnum) {
return false;
}
}

return true;
}

uint32_t last_updated_time_to_height( const fc::time_point& last_updated) {
const auto lib_num = controller.last_irreversible_block_num();
const auto lib_time = controller.last_irreversible_block_time();

uint32_t last_updated_height = lib_num;
if (last_updated > lib_time) {
const auto iter = time_to_block_num.find(last_updated);
EOS_ASSERT(iter != time_to_block_num.end(), chain::plugin_exception, "invalid block time encountered in on-chain accounts ${time}", ("time", last_updated));
last_updated_height = iter->second;
}

return last_updated_height;
}

/**
* Given a time_point, remove all permissions that were last updated at or after that time_point
* this will effectively remove any updates that happened at or after that time point
* Given a block number, remove all permissions that were last updated at or after that block number
* this will effectively roll back the database to just before the incoming block
*
* For each removed entry, this will create a new entry if there exists an equivalent {owner, name} permission
* at the HEAD state of the chain.
* @param bsp - the block to rollback before
*/
void rollback_to_before( const chain::block_state_ptr& bsp ) {
const auto t = bsp->block->timestamp.to_time_point();
auto& index = permission_info_index.get<by_last_updated>();
const auto bnum = bsp->block->block_num();
auto& index = permission_info_index.get<by_last_updated_height>();
const auto& permission_by_owner = controller.db().get_index<chain::permission_index>().indices().get<chain::by_owner>();

// roll back time-map
auto time_iter = time_to_block_num.rbegin();
while (time_iter != time_to_block_num.rend() && time_iter->second >= bnum) {
time_iter = decltype(time_iter){time_to_block_num.erase( std::next(time_iter).base() )};
}

auto curr_iter = index.rbegin();
while (!index.empty()) {
if (curr_iter == index.rend()) {
break;
}

const auto& pi = (*curr_iter);
if (pi.last_updated < t) {
if (pi.last_updated_height < bnum) {
break;
}

Expand All @@ -234,8 +265,11 @@ namespace eosio::chain_apis {
curr_iter = decltype(curr_iter)( index.erase(index.iterator_to(pi)) );
} else {
const auto& po = *itr;
index.modify(index.iterator_to(pi), [&po](auto& mutable_pi) {
mutable_pi.last_updated = po.last_updated;

uint32_t last_updated_height = po.last_updated == bsp->header.timestamp ? bsp->block_num : last_updated_time_to_height(po.last_updated);

index.modify(index.iterator_to(pi), [&po, last_updated_height](auto& mutable_pi) {
mutable_pi.last_updated_height = last_updated_height;
mutable_pi.threshold = po.auth.threshold;
});
add_to_bimaps(pi, po);
Expand Down Expand Up @@ -338,6 +372,11 @@ namespace eosio::chain_apis {
std::unique_lock write_lock(rw_mutex);

rollback_to_before(bsp);

// insert this blocks time into the time map
time_to_block_num.emplace(bsp->header.timestamp, bsp->block_num);

const auto bnum = bsp->block_num;
auto& index = permission_info_index.get<by_owner_name>();
const auto& permission_by_owner = controller.db().get_index<chain::permission_index>().indices().get<chain::by_owner>();

Expand All @@ -349,11 +388,11 @@ namespace eosio::chain_apis {
auto itr = index.find(key);
if (itr == index.end()) {
const auto& po = *source_itr;
itr = index.emplace(permission_info{ po.owner, po.name, po.last_updated, po.auth.threshold }).first;
itr = index.emplace(permission_info{ po.owner, po.name, bnum, po.auth.threshold }).first;
} else {
remove_from_bimaps(*itr);
index.modify(itr, [&](auto& mutable_pi){
mutable_pi.last_updated = source_itr->last_updated;
mutable_pi.last_updated_height = bnum;
mutable_pi.threshold = source_itr->auth.threshold;
});
}
Expand Down Expand Up @@ -447,6 +486,10 @@ namespace eosio::chain_apis {
cached_trace_map_t cached_trace_map; ///< temporary cache of uncommitted traces
onblock_trace_t onblock_trace; ///< temporary cache of on_block trace

using time_map_t = std::map<fc::time_point, uint32_t>;
time_map_t time_to_block_num;



using name_bimap_t = bimap<multiset_of<weighted<chain::permission_level>>, multiset_of<permission_info::cref>>;
using key_bimap_t = bimap<multiset_of<weighted<chain::public_key_type>>, multiset_of<permission_info::cref>>;
Expand Down
119 changes: 83 additions & 36 deletions plugins/chain_plugin/test/test_account_query_db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,60 +40,107 @@ BOOST_AUTO_TEST_SUITE(account_query_db_tests)

BOOST_FIXTURE_TEST_CASE(newaccount_test, TESTER) { try {

// instantiate an account_query_db
auto aq_db = account_query_db(*control);
// instantiate an account_query_db
auto aq_db = account_query_db(*control);

//link aq_db to the `accepted_block` signal on the controller
auto c2 = control->accepted_block.connect([&](const block_state_ptr& blk) {
auto c2 = control->accepted_block.connect([&](const block_state_ptr& blk) {
aq_db.commit_block( blk);
});
});

produce_blocks(10);
produce_blocks(10);

account_name tester_account = "tester"_n;
const auto trace_ptr = create_account(tester_account);
aq_db.cache_transaction_trace(trace_ptr);
produce_block();
account_name tester_account = "tester"_n;
const auto trace_ptr = create_account(tester_account);
aq_db.cache_transaction_trace(trace_ptr);
produce_block();

params pars;
pars.keys.emplace_back(get_public_key(tester_account, "owner"));
const auto results = aq_db.get_accounts_by_authorizers(pars);
params pars;
pars.keys.emplace_back(get_public_key(tester_account, "owner"));
const auto results = aq_db.get_accounts_by_authorizers(pars);

BOOST_TEST_REQUIRE(find_account_name(results, tester_account) == true);
BOOST_TEST_REQUIRE(find_account_name(results, tester_account) == true);

} FC_LOG_AND_RETHROW() }

BOOST_FIXTURE_TEST_CASE(updateauth_test, TESTER) { try {

// instantiate an account_query_db
auto aq_db = account_query_db(*control);
// instantiate an account_query_db
auto aq_db = account_query_db(*control);

//link aq_db to the `accepted_block` signal on the controller
auto c = control->accepted_block.connect([&](const block_state_ptr& blk) {
aq_db.commit_block( blk);
});
//link aq_db to the `accepted_block` signal on the controller
auto c = control->accepted_block.connect([&](const block_state_ptr& blk) {
aq_db.commit_block( blk);
});

produce_blocks(10);

produce_blocks(10);
const auto& tester_account = "tester"_n;
const string role = "first";
produce_block();
create_account(tester_account);

const auto& tester_account = "tester"_n;
const string role = "first";
produce_block();
create_account(tester_account);
const auto trace_ptr = push_action(config::system_account_name, updateauth::get_name(), tester_account, fc::mutable_variant_object()
("account", tester_account)
("permission", "role"_n)
("parent", "active")
("auth", authority(get_public_key(tester_account, role), 5))
);
aq_db.cache_transaction_trace(trace_ptr);
produce_block();

const auto trace_ptr = push_action(config::system_account_name, updateauth::get_name(), tester_account, fc::mutable_variant_object()
("account", tester_account)
("permission", "role"_n)
("parent", "active")
("auth", authority(get_public_key(tester_account, role), 5))
);
aq_db.cache_transaction_trace(trace_ptr);
produce_block();
params pars;
pars.keys.emplace_back(get_public_key(tester_account, role));
const auto results = aq_db.get_accounts_by_authorizers(pars);

params pars;
pars.keys.emplace_back(get_public_key(tester_account, role));
const auto results = aq_db.get_accounts_by_authorizers(pars);
BOOST_TEST_REQUIRE(find_account_auth(results, tester_account, "role"_n) == true);

} FC_LOG_AND_RETHROW() }

BOOST_TEST_REQUIRE(find_account_auth(results, tester_account, "role"_n) == true);
BOOST_AUTO_TEST_CASE(future_fork_test) { try {
tester node_a(setup_policy::none);
tester node_b(setup_policy::none);

// instantiate an account_query_db
auto aq_db = account_query_db(*node_a.control);

//link aq_db to the `accepted_block` signal on the controller
auto c = node_a.control->accepted_block.connect([&](const block_state_ptr& blk) {
aq_db.commit_block( blk);
});

// create 10 blocks synced
for (int i = 0; i < 10; i++) {
node_b.push_block(node_a.produce_block());
}

// produce a block on node A with a new account and permission
const auto& tester_account = "tester"_n;
const string role = "first";
node_a.create_account(tester_account);

const auto trace_ptr = node_a.push_action(config::system_account_name, updateauth::get_name(), tester_account, fc::mutable_variant_object()
("account", tester_account)
("permission", "role"_n)
("parent", "active")
("auth", authority(node_a.get_public_key(tester_account, role), 5))
);
aq_db.cache_transaction_trace(trace_ptr);
node_a.produce_block();

params pars;
pars.keys.emplace_back(node_a.get_public_key(tester_account, role));

const auto pre_results = aq_db.get_accounts_by_authorizers(pars);
BOOST_TEST_REQUIRE(find_account_auth(pre_results, tester_account, "role"_n) == true);

// have node B take over from head-1 and produce "future" blocks to overtake
node_a.push_block(node_b.produce_block(fc::milliseconds(config::block_interval_ms * 100)));
node_a.push_block(node_b.produce_block());

// ensure the account was forked away
const auto post_results = aq_db.get_accounts_by_authorizers(pars);
BOOST_TEST_REQUIRE(post_results.accounts.size() == 0);

} FC_LOG_AND_RETHROW() }

Expand Down

0 comments on commit 52ae351

Please sign in to comment.