Merge pull request #10195 from EOSIO/jjz-merge-security-fixes

Merge security fix PR#364 into release/2.1.x

plugins/chain_plugin/account_query_db.cpp

@@ -214,8 +214,13 @@ namespace eosio::chain_apis {
          auto& index = permission_info_index.get<by_last_updated>();
          const auto& permission_by_owner = controller.db().get_index<chain::permission_index>().indices().get<chain::by_owner>();
 
+         auto curr_iter = index.rbegin();
          while (!index.empty()) {
-            const auto& pi = (*index.rbegin());
+            if (curr_iter == index.rend()) {
+               break;
+            }
+
+            const auto& pi = (*curr_iter);
             if (pi.last_updated < t) {
                break;
             }
@@ -226,14 +231,15 @@ namespace eosio::chain_apis {
             auto itr = permission_by_owner.find(std::make_tuple(pi.owner, pi.name));
             if (itr == permission_by_owner.end()) {
                // this permission does not exist at this point in the chains history
-               index.erase(index.iterator_to(pi));
+               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;
                   mutable_pi.threshold = po.auth.threshold;
                });
                add_to_bimaps(pi, po);
+               ++curr_iter;
             }
          }
       }

plugins/chain_plugin/test/test_account_query_db.cpp

@@ -97,5 +97,99 @@ BOOST_FIXTURE_TEST_CASE(updateauth_test, TESTER) { try {
 
 } FC_LOG_AND_RETHROW() }
 
+BOOST_AUTO_TEST_CASE(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 auto& tester_account2 = "tester2"_n;
+      const string role = "first";
+      node_a.create_account(tester_account);
+      node_a.create_account(tester_account2);
+
+      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)), 1
+      );
+      aq_db.cache_transaction_trace(trace_ptr);
+      const auto trace_ptr2 = node_a.push_action(config::system_account_name, updateauth::get_name(), tester_account2, fc::mutable_variant_object()
+            ("account", tester_account2)
+            ("permission", "role"_n)
+            ("parent", "active")
+            ("auth",  authority(node_a.get_public_key(tester_account2, role), 5)), 2
+      );
+      aq_db.cache_transaction_trace(trace_ptr2);
+      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 also update permissions
+      node_b.create_account(tester_account);
+      node_b.create_account(tester_account2);
+
+      const auto trace_ptr3 = node_b.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_b.get_public_key(tester_account, role), 6)), 1
+      );
+      aq_db.cache_transaction_trace(trace_ptr3);
+      const auto trace_ptr4 = node_b.push_action(config::system_account_name, updateauth::get_name(), tester_account2, fc::mutable_variant_object()
+            ("account", tester_account2)
+            ("permission", "role"_n)
+            ("parent", "active")
+            ("auth",  authority(node_b.get_public_key(tester_account2, role), 6)), 2
+      );
+      aq_db.cache_transaction_trace(trace_ptr4);
+
+      // push b's onto a
+      node_a.push_block(node_b.produce_block());
+
+      const auto trace_ptr5 = node_b.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_b.get_public_key(tester_account, role), 5)), 3
+      );
+      aq_db.cache_transaction_trace(trace_ptr5);
+      const auto trace_ptr6 = node_b.push_action(config::system_account_name, updateauth::get_name(), tester_account2, fc::mutable_variant_object()
+            ("account", tester_account2)
+            ("permission", "role"_n)
+            ("parent", "active")
+            ("auth",  authority(node_b.get_public_key(tester_account2, role), 5)), 4
+      );
+      aq_db.cache_transaction_trace(trace_ptr6);
+
+      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);
+
+      // verify correct account is in results
+      BOOST_TEST_REQUIRE(post_results.accounts.size() == 1);
+
+   } FC_LOG_AND_RETHROW() }
+
+
 BOOST_AUTO_TEST_SUITE_END()