diff --git a/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp b/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp index 75564b46821..6b146f606f6 100644 --- a/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp @@ -48,8 +48,9 @@ namespace eosio { namespace chain { namespace webassembly { }; template struct is_whitelisted_type> { - // If expanding this beyond char, add a static_assert of the alignment of every - // additional type allowed. + // Currently only a span of [const] char is allowed so there are no alignment concerns. + // If we wish to expand to general span in the future, changes are needed in EOS VM + // to check proper alignment of the void* within from_wasm before constructing the span. static constexpr bool value = std::is_same_v, char>; }; template @@ -68,11 +69,31 @@ namespace eosio { namespace chain { namespace webassembly { inline static constexpr bool are_whitelisted_legacy_types_v = (... && detail::is_whitelisted_legacy_type::value); template - inline static bool is_aliasing(const T& a, const U& b) { - std::uintptr_t a_ui = reinterpret_cast(a.data()); - std::uintptr_t b_ui = reinterpret_cast(b.data()); - return a_ui < b_ui ? a_ui + a.size_bytes() > b_ui : b_ui + b.size_bytes() > a_ui; + inline static bool is_aliasing(const T& s1, const U& s2) { + std::uintptr_t a_begin = reinterpret_cast(s1.data()); + std::uintptr_t a_end = a_begin + s1.size_bytes(); + + std::uintptr_t b_begin = reinterpret_cast(s2.data()); + std::uintptr_t b_end = b_begin + s2.size_bytes(); + + // Aliasing iff the intersection of intervals [a_begin, a_end) and [b_begin, b_end) has non-zero size. + + if (a_begin > b_begin) { + std::swap(a_begin, b_begin); + std::swap(a_end, b_end); + } + + if (a_end <= b_begin) // intersection interval with non-zero size does not exist + return false; + + // Intersection interval is [b_begin, std::min(a_end, b_end)). + + if (std::min(a_end, b_end) == b_begin) // intersection interval has zero size + return false; + + return true; } + inline static bool is_nan( const float32_t f ) { return f32_is_nan( f ); } @@ -110,7 +131,10 @@ namespace eosio { namespace chain { namespace webassembly { namespace detail { template - vm::span to_span(const vm::argument_proxy& val) { return {val.get(), 1}; } + vm::span to_span(const vm::argument_proxy& val) { + return {static_cast(val.get_original_pointer()), sizeof(T)}; + } + template vm::span to_span(const vm::span& val) { return val; } } @@ -120,15 +144,10 @@ namespace eosio { namespace chain { namespace webassembly { using namespace eosio::vm; using arg_t = std::decay_t; static_assert( is_whitelisted_type_v, "whitelisted type violation"); - if constexpr (is_span_type_v) { - // check alignment while we are here - EOS_ASSERT( reinterpret_cast(arg.data()) % alignof(typename arg_t::value_type) == 0, - wasm_exception, "memory not aligned" ); - } if constexpr (is_span_type_v || vm::is_argument_proxy_type_v) { - eosio::vm::invoke_on([&](auto&& narg, auto&&... nrest) { + eosio::vm::invoke_on([&arg](auto&& narg, auto&&... nrest) { using nested_arg_t = std::decay_t; - if constexpr (eosio::vm::is_span_type_v || vm::is_argument_proxy_type_v) + if constexpr (eosio::vm::is_span_type_v || vm::is_argument_proxy_type_v) EOS_ASSERT(!is_aliasing(detail::to_span(arg), detail::to_span(narg)), wasm_exception, "pointers not allowed to alias"); }, rest...); } diff --git a/unittests/kv_tests.cpp b/unittests/kv_tests.cpp index 4a0b6d142a3..9c84ea41c20 100644 --- a/unittests/kv_tests.cpp +++ b/unittests/kv_tests.cpp @@ -733,13 +733,29 @@ FC_LOG_AND_RETHROW() // Must be set on a privileged account static const char kv_setup_wast[] = R"=====( (module + (func $action_data_size (import "env" "action_data_size") (result i32)) + (func $read_action_data (import "env" "read_action_data") (param i32 i32) (result i32)) (func $kv_set_parameters_packed (import "env" "set_kv_parameters_packed") (param i64 i32 i32)) (func $set_resource_limit (import "env" "set_resource_limit") (param i64 i64 i64)) (memory 1) (func (export "apply") (param i64 i64 i64) + (local $next_name_address i32) + (local $bytes_remaining i32) (call $kv_set_parameters_packed (get_local 2) (i32.const 0) (i32.const 16)) - (call $set_resource_limit (i64.const 11327368866104868864) (i64.const 5454140623722381312) (i64.const -1)) - (call $set_resource_limit (i64.const 11327368596746665984) (i64.const 5454140623722381312) (i64.const -1)) + (set_local $next_name_address (i32.const 16)) + (set_local $bytes_remaining (call $action_data_size)) + (if + (i32.ge_s (get_local $bytes_remaining) (i32.const 8)) + (then + (drop (call $read_action_data (get_local $next_name_address) (get_local $bytes_remaining))) + (loop + (call $set_resource_limit (i64.load (get_local $next_name_address)) (i64.const 5454140623722381312) (i64.const -1)) + (set_local $bytes_remaining (i32.sub (get_local $bytes_remaining) (i32.const 8))) + (set_local $next_name_address (i32.add (get_local $next_name_address) (i32.const 8))) + (br_if 0 (i32.ge_s (get_local $bytes_remaining) (i32.const 8))) + ) + ) + ) ) (data (i32.const 4) "\00\04\00\00") (data (i32.const 8) "\00\00\10\00") @@ -747,6 +763,16 @@ static const char kv_setup_wast[] = R"=====( ) )====="; +std::vector construct_names_payload( std::vector names ) { + std::vector result; + result.resize(names.size() * sizeof(name)); + fc::datastream ds(result.data(), result.size()); + for( auto n : names ) { + fc::raw::pack(ds, n); + } + return result; +} + // Call iterator intrinsics that create native state // and then notify another contract. The kv context should // not affect recipient. @@ -794,7 +820,7 @@ BOOST_DATA_TEST_CASE_F(tester, notify, bdata::make(databases), db) { create_accounts({ N(setup), N(notified), N(notify) }); set_code( N(setup), kv_setup_wast ); push_action( N(eosio), N(setpriv), N(eosio), mutable_variant_object()("account", N(setup))("is_priv", 1)); - BOOST_TEST_REQUIRE(push_action( action({}, N(setup), db, {}), N(setup).to_uint64_t() ) == ""); + BOOST_TEST_REQUIRE(push_action( action({}, N(setup), db, construct_names_payload({N(notified), N(notify)})), N(setup).to_uint64_t() ) == ""); set_code( N(notify), kv_notify_wast ); set_code( N(notified), kv_notified_wast ); @@ -802,4 +828,97 @@ BOOST_DATA_TEST_CASE_F(tester, notify, bdata::make(databases), db) { BOOST_TEST_REQUIRE(push_action( action({}, N(notify), db, {}), N(notify).to_uint64_t() ) == ""); } +// Check corner cases of alias checks for the kv_set and kv_get intrinsics +static const char kv_alias_pass_wast[] = R"=====( +(module + (func $kv_get (import "env" "kv_get") (param i64 i64 i32 i32 i32) (result i32)) + (func $kv_set (import "env" "kv_set") (param i64 i64 i32 i32 i32 i32) (result i64)) + (memory 1) + (func (export "apply") (param i64 i64 i64) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 1) (i32.const 1) (i32.const 0))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 1))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 2) (i32.const 1) (i32.const 0))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 2))) + (drop (call $kv_get (get_local 2) (get_local 0) (i32.const 1) (i32.const 1) (i32.const 2))) + (drop (call $kv_get (get_local 2) (get_local 0) (i32.const 2) (i32.const 0) (i32.const 2))) + (drop (call $kv_get (get_local 2) (get_local 0) (i32.const 6) (i32.const 1) (i32.const 2))) + (drop (call $kv_get (get_local 2) (get_local 0) (i32.const 6) (i32.const 0) (i32.const 2))) + (drop (call $kv_get (get_local 2) (get_local 0) (i32.const 4) (i32.const 0) (i32.const 2))) + ) +) +)====="; + +static const char kv_alias_general_wast[] = R"=====( +(module + (func $read_action_data (import "env" "read_action_data") (param i32 i32) (result i32)) + (func $kv_get (import "env" "kv_get") (param i64 i64 i32 i32 i32) (result i32)) + (func $kv_set (import "env" "kv_set") (param i64 i64 i32 i32 i32 i32) (result i64)) + (memory 1) + (func (export "apply") (param i64 i64 i64) + (local $span_start i32) + (local $span_size i32) + (drop (call $read_action_data (i32.const 0) (i32.const 8))) + (set_local $span_start (i32.load (i32.const 0))) + (set_local $span_size (i32.load (i32.const 4))) + (drop (call $kv_get (get_local 2) (get_local 0) (get_local $span_start) (get_local $span_size) (i32.const 32))) + (drop (call $kv_set (get_local 2) (get_local 0) (i32.const 64) (i32.const 4) (get_local $span_start) (get_local $span_size))) + (drop (call $kv_set (get_local 2) (get_local 0) (get_local $span_start) (get_local $span_size) (i32.const 128) (i32.const 4))) + ) +) +)====="; + +BOOST_DATA_TEST_CASE_F(tester, alias, bdata::make(databases), db) { + const name alias_pass_account{N(alias.pass)}; + const name alias_general_account{N(alias.gen)}; + + create_accounts({ N(setup), alias_pass_account, alias_general_account }); + set_code( N(setup), kv_setup_wast ); + push_action( N(eosio), N(setpriv), N(eosio), mutable_variant_object()("account", N(setup))("is_priv", 1)); + BOOST_TEST_REQUIRE(push_action( action({}, N(setup), db, construct_names_payload({alias_pass_account, alias_general_account})), N(setup).to_uint64_t() ) == ""); + + set_code( alias_pass_account, kv_alias_pass_wast ); + set_code( alias_general_account, kv_alias_general_wast ); + + BOOST_TEST_CHECK(push_action( action({}, alias_pass_account, db, {}), alias_pass_account.to_uint64_t() ) == ""); + + auto construct_span_payload = [](uint32_t span_start, uint32_t span_size) -> std::vector { + std::vector result; + result.resize(8); + fc::datastream ds(result.data(), result.size()); + fc::raw::pack(ds, span_start); + fc::raw::pack(ds, span_size); + return result; + }; + + // The kv_alias_general_wast code effectively creates three reserved intervals in linear memory: [32, 36), [64, 68), and [128, 132). + // The constructed span sent as an argument to the code can be used to test various aliasing scenarios. + + static const char* const alias_error_msg = "pointers not allowed to alias"; + + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(31, 1)), alias_general_account.to_uint64_t() ) == ""); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(36, 28)), alias_general_account.to_uint64_t() ) == ""); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(68, 60)), alias_general_account.to_uint64_t() ) == ""); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(132, 1)), alias_general_account.to_uint64_t() ) == ""); + + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(31, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(32, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(33, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(35, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(35, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); + + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(63, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(64, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(65, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(67, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(67, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); + + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(127, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(128, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(129, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(131, 1)), alias_general_account.to_uint64_t() ) == alias_error_msg); + BOOST_TEST_CHECK(push_action( action({}, alias_general_account, db, construct_span_payload(131, 2)), alias_general_account.to_uint64_t() ) == alias_error_msg); +} + BOOST_AUTO_TEST_SUITE_END()