fix #2208 and add related unit tests

contracts/test_api/test_api.hpp

@@ -17,11 +17,6 @@ namespace eosio {
 // NOTE: including eosiolib/transaction.hpp here causes !"unresolvable": env._ZNKSt3__120__vector_base_commonILb1EE20__throw_length_errorEv
 //       errors in api_tests/memory_tests
 
-static constexpr unsigned long long WASM_TEST_ACTION(const char* cls, const char* method)
-{
-  return static_cast<unsigned long long>(DJBH(cls)) << 32 | static_cast<unsigned long long>(DJBH(method));
-}
-
 #define WASM_TEST_HANDLER(CLASS, METHOD) \
   if( action == WASM_TEST_ACTION(#CLASS, #METHOD) ) { \
      CLASS::METHOD(); \
@@ -93,6 +88,8 @@ struct test_db {
    static void idx64_general(uint64_t receiver, uint64_t code, uint64_t action);
    static void idx64_lowerbound(uint64_t receiver, uint64_t code, uint64_t action);
    static void idx64_upperbound(uint64_t receiver, uint64_t code, uint64_t action);
+
+   static void test_invalid_access(uint64_t receiver, uint64_t code, uint64_t action);
 };
 
 struct test_multi_index {

contracts/test_api/test_api_common.hpp

@@ -18,6 +18,11 @@ static constexpr unsigned int DJBH(const char* cp)
   return hash;
 }
 
+static constexpr unsigned long long WASM_TEST_ACTION(const char* cls, const char* method)
+{
+  return static_cast<unsigned long long>(DJBH(cls)) << 32 | static_cast<unsigned long long>(DJBH(method));
+}
+
 #pragma pack(push, 1)
 struct dummy_action {
    static uint64_t get_name() {
@@ -59,3 +64,12 @@ static_assert( sizeof(u128_action) == 16*3 , "unexpected packing" );
 #define DUMMY_ACTION_DEFAULT_A 0x45
 #define DUMMY_ACTION_DEFAULT_B 0xab11cd1244556677
 #define DUMMY_ACTION_DEFAULT_C 0x7451ae12
+
+struct invalid_access_action {
+   uint64_t code;
+   uint64_t val;
+   uint32_t index;
+   bool store;
+
+   EOSLIB_SERIALIZE( invalid_access_action, (code)(val)(index)(store) )
+};

contracts/test_api_db/test_api_db.cpp

@@ -9,13 +9,14 @@
 
 extern "C" {
    void apply( uint64_t receiver, uint64_t code, uint64_t action ) {
-      require_auth(code); 
+      require_auth(code);
       WASM_TEST_HANDLER_EX(test_db, primary_i64_general);
       WASM_TEST_HANDLER_EX(test_db, primary_i64_lowerbound);
       WASM_TEST_HANDLER_EX(test_db, primary_i64_upperbound);
       WASM_TEST_HANDLER_EX(test_db, idx64_general);
       WASM_TEST_HANDLER_EX(test_db, idx64_lowerbound);
       WASM_TEST_HANDLER_EX(test_db, idx64_upperbound);
+      WASM_TEST_HANDLER_EX(test_db, test_invalid_access);
 
       //unhandled test call
       eosio_assert(false, "Unknown Test");

contracts/test_api_db/test_db.cpp

 #include <eosiolib/types.hpp>
 #include <eosiolib/action.hpp>
+#include <eosiolib/transaction.hpp>
+#include <eosiolib/datastream.hpp>
 #include <eosiolib/db.h>
 #include <eosiolib/memory.hpp>
 #include "../test_api/test_api.hpp"
@@ -433,3 +435,63 @@ void test_db::idx64_upperbound(uint64_t receiver, uint64_t code, uint64_t action
       eosio_assert(ub < 0, err.c_str());
    }
 }
+
+void test_db::test_invalid_access(uint64_t receiver, uint64_t code, uint64_t action)
+{
+   (void)code;(void)action;
+   auto act = eosio::get_action(1, 0);
+   auto ia = eosio::unpack<invalid_access_action>(act.data);
+   uint64_t scope = N(access);
+   uint64_t table = scope;
+   uint64_t pk    = scope;
+
+   int32_t itr = -1;
+   uint64_t value;
+   switch( ia.index ) {
+      case 1:
+         itr = db_idx64_find_primary(ia.code, scope, table, &value, pk);
+      break;
+      case 0:
+      default:
+         itr = db_find_i64(ia.code, scope, table, pk);
+      break;
+   }
+   if( ia.store ) {
+      uint64_t value_to_store = ia.val;
+      if( itr < 0 ) {
+         switch( ia.index ) {
+            case 1:
+               db_idx64_store( scope, table, receiver, pk, &value_to_store );
+            break;
+            case 0:
+            default:
+               db_store_i64( scope, table, receiver, pk, &value_to_store, sizeof(value_to_store) );
+            break;
+         }
+      } else {
+         switch( ia.index ) {
+            case 1:
+               db_idx64_update( itr, receiver, &value_to_store);
+            break;
+            case 0:
+            default:
+               db_update_i64( itr, receiver, &value_to_store, sizeof(value_to_store) );
+            break;
+         }
+      }
+      //eosio::print("test_invalid_access: stored ", value_to_store, "\n");
+   } else {
+      eosio_assert( itr >= 0, "test_invalid_access: could not find row" );
+      switch( ia.index ) {
+         case 1:
+         break;
+         case 0:
+         default:
+            eosio_assert( db_get_i64( itr, &value, sizeof(value) ) == sizeof(value),
+                          "test_invalid_access: value in primary table was incorrect size" );
+         break;
+      }
+      //eosio::print("test_invalid_access: expected ", ia.val, " and retrieved ", value, "\n");
+      eosio_assert( value == ia.val, "test_invalid_access: value did not match" );
+   }
+}

libraries/chain/apply_context.cpp

@@ -473,8 +473,10 @@ int apply_context::db_store_i64( uint64_t code, uint64_t scope, uint64_t table,
 void apply_context::db_update_i64( int iterator, account_name payer, const char* buffer, size_t buffer_size ) {
    const key_value_object& obj = keyval_cache.get( iterator );
 
-   const auto& tab = keyval_cache.get_table( obj.t_id );
-   require_write_lock( tab.scope );
+   const auto& table_obj = keyval_cache.get_table( obj.t_id );
+   FC_ASSERT( table_obj.code == receiver, "db access violation" );
+
+   require_write_lock( table_obj.scope );
 
    const int64_t overhead = config::billable_size_v<key_value_object>;
    int64_t old_size = (int64_t)(obj.value.size() + overhead);
@@ -501,11 +503,14 @@ void apply_context::db_update_i64( int iterator, account_name payer, const char*
 
 void apply_context::db_remove_i64( int iterator ) {
    const key_value_object& obj = keyval_cache.get( iterator );
-   update_db_usage( obj.payer,  -(obj.value.size() + config::billable_size_v<key_value_object>) );
 
    const auto& table_obj = keyval_cache.get_table( obj.t_id );
+   FC_ASSERT( table_obj.code == receiver, "db access violation" );
+
    require_write_lock( table_obj.scope );
 
+   update_db_usage( obj.payer,  -(obj.value.size() + config::billable_size_v<key_value_object>) );
+
    mutable_db.modify( table_obj, [&]( auto& t ) {
       --t.count;
    });

libraries/chain/include/eosio/chain/apply_context.hpp

@@ -211,6 +211,8 @@ class apply_context {
                context.update_db_usage( obj.payer, -( config::billable_size_v<ObjectType> ) );
 
                const auto& table_obj = itr_cache.get_table( obj.t_id );
+               FC_ASSERT( table_obj.code == context.receiver, "db access violation" );
+
                context.require_write_lock( table_obj.scope );
 
                context.mutable_db.modify( table_obj, [&]( auto& t ) {
@@ -228,7 +230,10 @@ class apply_context {
             void update( int iterator, account_name payer, secondary_key_proxy_const_type secondary ) {
                const auto& obj = itr_cache.get( iterator );
 
-               context.require_write_lock( itr_cache.get_table( obj.t_id ).scope );
+               const auto& table_obj = itr_cache.get_table( obj.t_id );
+               FC_ASSERT( table_obj.code == context.receiver, "db access violation" );
+
+               context.require_write_lock( table_obj.scope );
 
                if( payer == account_name() ) payer = obj.payer;
 

tests/api_tests/api_tests.cpp

@@ -46,9 +46,10 @@
 #define DISABLE_EOSLIB_SERIALIZE
 #include <test_api/test_api_common.hpp>
 
-FC_REFLECT( dummy_action, (a)(b)(c) );
-FC_REFLECT( u128_action, (values) );
-FC_REFLECT( cf_action, (payload)(cfd_idx) );
+FC_REFLECT( dummy_action, (a)(b)(c) )
+FC_REFLECT( u128_action, (values) )
+FC_REFLECT( cf_action, (payload)(cfd_idx) )
+FC_REFLECT( invalid_access_action, (code)(val)(index)(store) )
 
 #ifdef NON_VALIDATING_TEST
 #define TESTER tester
@@ -880,18 +881,69 @@ BOOST_FIXTURE_TEST_CASE(chain_tests, TESTER) { try {
  * db_tests test case
  *************************************************************************************/
 BOOST_FIXTURE_TEST_CASE(db_tests, TESTER) { try {
-	produce_blocks(2);
-	create_account( N(testapi) );
-	produce_blocks(1000);
-	set_code( N(testapi), test_api_db_wast );
-	produce_blocks(1);
+   produce_blocks(2);
+   create_account( N(testapi) );
+   create_account( N(testapi2) );
+   produce_blocks(1000);
+   set_code( N(testapi), test_api_db_wast );
+   set_code( N(testapi2), test_api_db_wast );
+   produce_blocks(1);
 
-	CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_general", {});
-	CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_lowerbound", {});
-	CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_upperbound", {});
-	CALL_TEST_FUNCTION( *this, "test_db", "idx64_general", {});
-	CALL_TEST_FUNCTION( *this, "test_db", "idx64_lowerbound", {});
-	CALL_TEST_FUNCTION( *this, "test_db", "idx64_upperbound", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_general", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_lowerbound", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "primary_i64_upperbound", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "idx64_general", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "idx64_lowerbound", {});
+   CALL_TEST_FUNCTION( *this, "test_db", "idx64_upperbound", {});
+
+   // Store value in primary table
+   invalid_access_action ia1{.code = N(testapi), .val = 10, .index = 0, .store = true};
+   auto res = push_action( action({{N(testapi), config::active_name}},
+                                  N(testapi), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                                  fc::raw::pack(ia1)),
+                           N(testapi) );
+   BOOST_CHECK_EQUAL( res, success() );
+
+   // Attempt to change the value stored in the primary table under the code of N(testapi)
+   invalid_access_action ia2{.code = ia1.code, .val = 20, .index = 0, .store = true};
+   res = push_action( action({{N(testapi2), config::active_name}},
+                             N(testapi2), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                             fc::raw::pack(ia2)),
+                      N(testapi2) );
+   BOOST_CHECK_EQUAL( boost::algorithm::ends_with(res, "db access violation"), true );
+
+
+   // Verify that the value has not changed.
+   ia1.store = false;
+   res = push_action( action({{N(testapi), config::active_name}},
+                             N(testapi), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                             fc::raw::pack(ia1)),
+                      N(testapi) );
+   BOOST_CHECK_EQUAL( res, success() );
+
+   // Store value in secondary table
+   ia1.store = true; ia1.index = 1;
+   res = push_action( action({{N(testapi), config::active_name}},
+                             N(testapi), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                             fc::raw::pack(ia1)),
+                      N(testapi) );
+   BOOST_CHECK_EQUAL( res, success() );
+
+   // Attempt to change the value stored in the secondary table under the code of N(testapi)
+   ia2.index = 1;
+   res = push_action( action({{N(testapi2), config::active_name}},
+                             N(testapi2), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                             fc::raw::pack(ia2)),
+                      N(testapi2) );
+   BOOST_CHECK_EQUAL( boost::algorithm::ends_with(res, "db access violation"), true );
+
+   // Verify that the value has not changed.
+   ia1.store = false;
+   res = push_action( action({{N(testapi), config::active_name}},
+                             N(testapi), WASM_TEST_ACTION("test_db", "test_invalid_access"),
+                             fc::raw::pack(ia1)),
+                      N(testapi) );
+   BOOST_CHECK_EQUAL( res, success() );
 
    BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW() }