Merge pull request eosnetworkfoundation#726 from eosnetworkfoundation…

…/elmato/restrict-accepted-rlp-format-transaction [1.0] Restrict rlp formats when decoding transaction
ultraio · Jun 7, 2024 · 2bfafa8 · 2bfafa8
2 parents 9aa30b3 + d6f291b
commit 2bfafa8
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 5 deletions.
diff --git a/include/evm_runtime/transaction.hpp b/include/evm_runtime/transaction.hpp
@@ -32,7 +32,7 @@ struct transaction {
       eosio::check(rlptx_.has_value(), "no rlptx");
       ByteView bv{(const uint8_t*)rlptx_->data(), rlptx_->size()};
       silkworm::Transaction tmp;
-      eosio::check(silkworm::rlp::decode_transaction(bv, tmp, silkworm::rlp::Eip2718Wrapping::kBoth) && bv.empty(), "unable to decode transaction");
+      eosio::check(silkworm::rlp::decode_transaction(bv, tmp, silkworm::rlp::Eip2718Wrapping::kNone) && bv.empty(), "unable to decode transaction");
       tx_.emplace(tmp);
     }
     return tx_.value();
@@ -50,4 +50,4 @@ struct transaction {
   mutable std::optional<silkworm::Transaction> tx_;
 };
 
-} //namespace evm_runtime
+} //namespace evm_runtime
diff --git a/src/test_actions.cpp b/src/test_actions.cpp
@@ -24,7 +24,7 @@ using namespace silkworm;
     if(orlptx) {
         Transaction tx;
         ByteView bv{(const uint8_t*)orlptx->data(), orlptx->size()};
-        eosio::check(rlp::decode(bv,tx) && bv.empty(), "unable to decode transaction");
+        eosio::check(rlp::decode_transaction(bv, tx, rlp::Eip2718Wrapping::kString) && bv.empty(), "unable to decode transaction");
 
         runtime_config rc {
             .allow_special_signature = false,
@@ -378,4 +378,4 @@ using namespace silkworm;
     }
 }
 
-}
+}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -22,6 +22,7 @@ include_directories(
 )
 
 add_eosio_test_executable( unit_test
+    ${CMAKE_SOURCE_DIR}/rlp_encoding_tests.cpp
     ${CMAKE_SOURCE_DIR}/different_gas_token_tests.cpp
     ${CMAKE_SOURCE_DIR}/version_tests.cpp
     ${CMAKE_SOURCE_DIR}/account_id_tests.cpp

diff --git a/tests/basic_evm_tester.cpp b/tests/basic_evm_tester.cpp
@@ -440,7 +440,7 @@ transaction_trace_ptr basic_evm_tester::assertnonce(name account, uint64_t next_
 transaction_trace_ptr basic_evm_tester::pushtx(const silkworm::Transaction& trx, name miner, std::optional<uint64_t> min_inclusion_price)
 {
    silkworm::Bytes rlp;
-   silkworm::rlp::encode(rlp, trx);
+   silkworm::rlp::encode(rlp, trx, false);
 
    bytes rlp_bytes;
    rlp_bytes.resize(rlp.size());

diff --git a/tests/rlp_encoding_tests.cpp b/tests/rlp_encoding_tests.cpp
@@ -0,0 +1,55 @@
+#include "basic_evm_tester.hpp"
+#include <silkworm/core/execution/address.hpp>
+
+using namespace evm_test;
+struct rlp_encoding_tester : basic_evm_tester {
+   rlp_encoding_tester() {
+      create_accounts({"alice"_n});
+      transfer_token(faucet_account_name, "alice"_n, make_asset(10000'0000));
+      init();
+   }
+
+    transaction_trace_ptr my_pushtx(const silkworm::Transaction& trx, bool wrap_eip2718_into_string)
+    {
+        silkworm::Bytes rlp;
+        silkworm::rlp::encode(rlp, trx, wrap_eip2718_into_string);
+
+        bytes rlp_bytes;
+        rlp_bytes.resize(rlp.size());
+        memcpy(rlp_bytes.data(), rlp.data(), rlp.size());
+
+        return push_action(evm_account_name, "pushtx"_n, evm_account_name, mvo()("miner", evm_account_name)("rlptx", rlp_bytes));
+    }
+
+};
+
+BOOST_AUTO_TEST_SUITE(rlp_encoding_tests)
+
+BOOST_FIXTURE_TEST_CASE(wrap_tx_in_string, rlp_encoding_tester) try {
+
+    setversion(1, evm_account_name);
+    produce_blocks(2);
+
+    // Fund evm1 address with 10.0000 EOS (triggers version 1 change)
+    evm_eoa evm1;
+    transfer_token("alice"_n, evm_account_name, make_asset(10'0000), evm1.address_0x());
+
+    evm_eoa evm2;
+
+    auto tx = generate_tx(evm2.address, 1);
+    tx.type = silkworm::TransactionType::kDynamicFee;
+    tx.max_priority_fee_per_gas = 0;
+    tx.max_fee_per_gas = suggested_gas_price;
+
+    evm1.sign(tx);
+
+    // Wrap kDynamicFee tx in string
+    BOOST_REQUIRE_EXCEPTION(my_pushtx(tx, true), eosio_assert_message_exception,
+        [](const eosio_assert_message_exception& e) {return testing::expect_assert_message(e, "unable to decode transaction");});
+
+    // Don't wrap kDynamicFee tx in string
+    BOOST_CHECK_NO_THROW(my_pushtx(tx, false));
+
+} FC_LOG_AND_RETHROW()
+
+BOOST_AUTO_TEST_SUITE_END()