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

Commit

Permalink
Merge pull request #8996 from EOSIO/kv-database-fix-aliasing
Browse files Browse the repository at this point in the history 
Fix aliasing issues with new host function system
  • Loading branch information
@arhag
arhag authored Apr 24, 2020
2 parents 05b7a94 + 455b157 commit b4b04b5
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 17 deletions.
47 changes: 33 additions & 14 deletions libraries/chain/include/eosio/chain/webassembly/preconditions.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ namespace eosio { namespace chain { namespace webassembly {
};
template <typename T>
struct is_whitelisted_type<vm::span<T>> {
// 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<T> 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<std::remove_const_t<T>, char>;
};
template <typename T>
Expand All @@ -68,11 +69,31 @@ namespace eosio { namespace chain { namespace webassembly {
inline static constexpr bool are_whitelisted_legacy_types_v = (... && detail::is_whitelisted_legacy_type<Ts>::value);

template <typename T, typename U>
inline static bool is_aliasing(const T& a, const U& b) {
std::uintptr_t a_ui = reinterpret_cast<std::uintptr_t>(a.data());
std::uintptr_t b_ui = reinterpret_cast<std::uintptr_t>(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<std::uintptr_t>(s1.data());
std::uintptr_t a_end = a_begin + s1.size_bytes();

std::uintptr_t b_begin = reinterpret_cast<std::uintptr_t>(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 );
}
Expand Down Expand Up @@ -110,7 +131,10 @@ namespace eosio { namespace chain { namespace webassembly {

namespace detail {
template<typename T>
vm::span<const T> to_span(const vm::argument_proxy<T*>& val) { return {val.get(), 1}; }
vm::span<const char> to_span(const vm::argument_proxy<T*>& val) {
return {static_cast<const char*>(val.get_original_pointer()), sizeof(T)};
}

template<typename T>
vm::span<T> to_span(const vm::span<T>& val) { return val; }
}
Expand All @@ -120,15 +144,10 @@ namespace eosio { namespace chain { namespace webassembly {
using namespace eosio::vm;
using arg_t = std::decay_t<decltype(arg)>;
static_assert( is_whitelisted_type_v<arg_t>, "whitelisted type violation");
if constexpr (is_span_type_v<arg_t>) {
// check alignment while we are here
EOS_ASSERT( reinterpret_cast<std::uintptr_t>(arg.data()) % alignof(typename arg_t::value_type) == 0,
wasm_exception, "memory not aligned" );
}
if constexpr (is_span_type_v<arg_t> || vm::is_argument_proxy_type_v<arg_t>) {
eosio::vm::invoke_on<false, eosio::vm::invoke_on_all_t>([&](auto&& narg, auto&&... nrest) {
eosio::vm::invoke_on<false, eosio::vm::invoke_on_all_t>([&arg](auto&& narg, auto&&... nrest) {
using nested_arg_t = std::decay_t<decltype(narg)>;
if constexpr (eosio::vm::is_span_type_v<nested_arg_t> || vm::is_argument_proxy_type_v<arg_t>)
if constexpr (eosio::vm::is_span_type_v<nested_arg_t> || vm::is_argument_proxy_type_v<nested_arg_t>)
EOS_ASSERT(!is_aliasing(detail::to_span(arg), detail::to_span(narg)), wasm_exception, "pointers not allowed to alias");
}, rest...);
}
Expand Down
125 changes: 122 additions & 3 deletions unittests/kv_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,20 +733,46 @@ 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")
(data (i32.const 12) "\80\00\00\00")
)
)=====";

std::vector<char> construct_names_payload( std::vector<name> names ) {
std::vector<char> result;
result.resize(names.size() * sizeof(name));
fc::datastream<char*> 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.
Expand Down Expand Up @@ -794,12 +820,105 @@ 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 );

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<char> {
std::vector<char> result;
result.resize(8);
fc::datastream<char*> 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()

0 comments on commit b4b04b5

Please sign in to comment.