Release 2.6.1. (#537)

* Fix: Don't allow fee stealing. * Fix: Don't allow bridge receipt forging. * Fix(engine): Correctly account for changes in total supply of ETH on Aurora (#536) Co-authored-by: Michael Birch <[email protected]>
aurora-is-near · Jun 23, 2022 · 7109e30 · 7109e30
1 parent 52fb413
commit 7109e30
Show file tree

Hide file tree

Showing 25 changed files with 354 additions and 70 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [2.6.1] 2022-06-23
+
+### Fixes
+
+- Fixed an issue with accounting being problematic with the total supply of ETH on Aurora as it could artificially deplete by @[birchmd]. ([#536])
+- Fixed the possibility of forging receipts to allow for withdrawals on the Rainbow Bridge by [@birchmd], [@mfornet], [@sept-en] and [@joshuajbouw]. Written by [@birchmd].
+- Fixed the ability the steal funds from those by setting a fee when receiving NEP-141 as ERC-20 by [@birchmd], [@mfornet], and [@joshuajbouw]. Written by [@joshuajbouw].
+
+[#536]: https://github.com/aurora-is-near/aurora-engine/pull/536
+
 ## [2.6.0] 2022-06-08
 
 ### Added
@@ -271,7 +281,8 @@ struct SubmitResult {
 
 ## [1.0.0] - 2021-05-12
 
-[Unreleased]: https://github.com/aurora-is-near/aurora-engine/compare/2.6.0...develop
+[Unreleased]: https://github.com/aurora-is-near/aurora-engine/compare/2.6.1...develop
+[2.6.1]: https://github.com/aurora-is-near/aurora-engine/compare/2.6.0...2.6.1 
 [2.6.0]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.3...2.6.0
 [2.5.3]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.2...2.5.3
 [2.5.2]: https://github.com/aurora-is-near/aurora-engine/compare/2.5.1...2.5.2

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/README.md b/README.md
@@ -14,9 +14,9 @@ documentation.
 
 Network | Contract ID         | Chain ID   | Version
 ------- | ------------------- | ---------- | ------
-Mainnet | [`aurora`][Mainnet] | 1313161554 | 2.5.3
-Testnet | [`aurora`][Testnet] | 1313161555 | 2.6.0
-Local   | `aurora.test.near`  | 1313161556 | 2.6.0
+Mainnet | [`aurora`][Mainnet] | 1313161554 | 2.6.1
+Testnet | [`aurora`][Testnet] | 1313161555 | 2.6.1
+Local   | `aurora.test.near`  | 1313161556 | 2.6.1
 
 [Mainnet]: https://explorer.near.org/accounts/aurora
 [Testnet]: https://explorer.testnet.near.org/accounts/aurora

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-2.6.0
+2.6.1
diff --git a/engine-sdk/Cargo.toml b/engine-sdk/Cargo.toml
@@ -22,3 +22,5 @@ sha2 = { version = "0.9.3", default-features = false }
 std = ["aurora-engine-types/std"]
 contract = []
 log = []
+mainnet = []
+testnet = []
diff --git a/engine-sdk/src/near_runtime.rs b/engine-sdk/src/near_runtime.rs
@@ -6,6 +6,18 @@ use aurora_engine_types::parameters::{PromiseAction, PromiseBatchAction, Promise
 use aurora_engine_types::types::PromiseResult;
 use aurora_engine_types::H256;
 
+#[cfg(feature = "mainnet")]
+/// The mainnet eth_custodian address 0x6BFaD42cFC4EfC96f529D786D643Ff4A8B89FA52
+const CUSTODIAN_ADDRESS: &[u8] = &[
+    107, 250, 212, 44, 252, 78, 252, 150, 245, 41, 215, 134, 214, 67, 255, 74, 139, 137, 250, 82,
+];
+
+#[cfg(feature = "testnet")]
+/// The testnet eth_custodian address 0x84a82Bb39c83989D5Dc07e1310281923D2544dC2
+const CUSTODIAN_ADDRESS: &[u8] = &[
+    132, 168, 43, 179, 156, 131, 152, 157, 93, 192, 126, 19, 16, 40, 25, 35, 210, 84, 77, 194,
+];
+
 /// Wrapper type for indices in NEAR's register API.
 pub struct RegisterIndex(u64);
 
@@ -113,6 +125,10 @@ impl crate::io::IO for Runtime {
 
     fn return_output(&mut self, value: &[u8]) {
         unsafe {
+            #[cfg(any(feature = "mainnet", feature = "testnet"))]
+            if value.len() >= 56 && &value[36..56] == CUSTODIAN_ADDRESS {
+                panic!("ERR_ILLEGAL_RETURN");
+            }
             exports::value_return(value.len() as u64, value.as_ptr() as u64);
         }
     }

diff --git a/engine-tests/src/test_utils/mod.rs b/engine-tests/src/test_utils/mod.rs
@@ -292,6 +292,8 @@ impl AuroraRunner {
                 .unwrap_or_default();
             current_ft.total_eth_supply_on_near =
                 current_ft.total_eth_supply_on_near + NEP141Wei::new(init_balance.raw().as_u128());
+            current_ft.total_eth_supply_on_aurora = current_ft.total_eth_supply_on_aurora
+                + NEP141Wei::new(init_balance.raw().as_u128());
             current_ft
         };
 
@@ -315,7 +317,9 @@ impl AuroraRunner {
         );
 
         trie.insert(balance_key.to_vec(), balance_value.to_vec());
-        trie.insert(nonce_key.to_vec(), nonce_value.to_vec());
+        if !init_nonce.is_zero() {
+            trie.insert(nonce_key.to_vec(), nonce_value.to_vec());
+        }
         trie.insert(ft_key, ft_value.try_to_vec().unwrap());
         trie.insert(proof_key, vec![0]);
         trie.insert(

diff --git a/engine-tests/src/test_utils/one_inch/mod.rs b/engine-tests/src/test_utils/one_inch/mod.rs
@@ -15,7 +15,17 @@ pub(crate) fn download_and_compile_solidity_sources(
         // if multiple tests running in parallel saw `contracts_dir` does not exist).
         download_once.call_once(|| {
             let url = format!("https://github.com/1inch/{}", repo_name);
-            git2::Repository::clone(&url, &sources_dir).unwrap();
+            let repo = git2::Repository::clone(&url, &sources_dir).unwrap();
+            if repo_name == "limit-order-protocol" {
+                // We need to checkout a specific commit because the code in the current `master`
+                // cannot be used with our version of `ethereum-types`, it gives the following error:
+                // Error("unknown variant `error`, expected one of `constructor`, `function`, `event`, `fallback`, `receive`", line: 9, column: 21)
+                let commit_hash =
+                    git2::Oid::from_str("49ab85b3c39d916711495596a1bf811848437896").unwrap();
+                repo.set_head_detached(commit_hash).unwrap();
+                let mut opts = git2::build::CheckoutBuilder::new();
+                repo.checkout_head(Some(opts.force())).unwrap();
+            }
         });
     }
 

diff --git a/engine-tests/src/test_utils/standalone/mocks/mod.rs b/engine-tests/src/test_utils/standalone/mocks/mod.rs
@@ -1,10 +1,12 @@
 use crate::test_utils;
 use aurora_engine::engine;
 use aurora_engine::fungible_token::FungibleTokenMetadata;
-use aurora_engine::parameters::{FinishDepositCallArgs, InitCallArgs, NewCallArgs};
+use aurora_engine::parameters::{
+    FinishDepositCallArgs, InitCallArgs, NEP141FtOnTransferArgs, NewCallArgs,
+};
 use aurora_engine_sdk::env::{Env, DEFAULT_PREPAID_GAS};
 use aurora_engine_sdk::io::IO;
-use aurora_engine_types::types::{Address, NEP141Wei, NearGas, Wei};
+use aurora_engine_types::types::{Address, Balance, NEP141Wei, NearGas, Wei};
 use aurora_engine_types::{account_id::AccountId, H256, U256};
 use engine_standalone_storage::{BlockMetadata, Storage};
 use near_sdk_sim::DEFAULT_GAS;
@@ -96,7 +98,6 @@ pub fn mint_evm_account<I: IO + Copy, E: Env>(
         storage: std::iter::empty(),
         reset_storage: false,
     };
-    engine.apply(std::iter::once(state_change), std::iter::empty(), false);
 
     let deposit_args = FinishDepositCallArgs {
         new_owner_id: aurora_account_id.clone(),
@@ -114,8 +115,8 @@ pub fn mint_evm_account<I: IO + Copy, E: Env>(
     );
     io.remove_storage(&proof_key);
 
-    aurora_engine::connector::EthConnectorContract::init_instance(io)
-        .unwrap()
+    let mut connector = aurora_engine::connector::EthConnectorContract::init_instance(io).unwrap();
+    connector
         .finish_deposit(
             aurora_account_id.clone(),
             aurora_account_id.clone(),
@@ -124,6 +125,21 @@ pub fn mint_evm_account<I: IO + Copy, E: Env>(
         )
         .map_err(unsafe_to_string)
         .unwrap();
+
+    let transfer_args = NEP141FtOnTransferArgs {
+        sender_id: aurora_account_id.clone(),
+        amount: Balance::new(balance.raw().as_u128()),
+        msg: format!(
+            "aurora:{}{}",
+            hex::encode(Wei::zero().to_bytes()),
+            hex::encode(address.as_bytes())
+        ),
+    };
+    connector
+        .ft_on_transfer(&mut engine, &transfer_args)
+        .unwrap();
+
+    engine.apply(std::iter::once(state_change), std::iter::empty(), false);
 }
 
 pub fn unsafe_to_string<E: AsRef<[u8]>>(e: E) -> String {

diff --git a/engine-tests/src/tests/erc20_connector.rs b/engine-tests/src/tests/erc20_connector.rs
@@ -297,6 +297,7 @@ fn test_ft_on_transfer_fail() {
     assert_eq!(res, format!("\"{}\"", amount.to_string()));
 }
 
+#[ignore]
 #[test]
 fn test_relayer_charge_fee() {
     let mut runner = AuroraRunner::new();

diff --git a/engine-tests/src/tests/ghsa_3p69_m8gg_fwmf.rs b/engine-tests/src/tests/ghsa_3p69_m8gg_fwmf.rs
@@ -0,0 +1,48 @@
+use crate::test_utils;
+use borsh::BorshSerialize;
+
+#[test]
+fn test_exploit_fix() {
+    let (mut runner, mut signer, _) = crate::tests::sanity::initialize_transfer();
+
+    let constructor = test_utils::solidity::ContractConstructor::compile_from_source(
+        "src/tests/res",
+        "target/solidity_build",
+        "echo.sol",
+        "Echo",
+    );
+
+    let nonce = signer.use_nonce();
+    let contract = runner.deploy_contract(
+        &signer.secret_key,
+        |c| c.deploy_without_constructor(nonce.into()),
+        constructor,
+    );
+
+    let eth_custodian_address = if cfg!(feature = "mainnet-test") {
+        "6bfad42cfc4efc96f529d786d643ff4a8b89fa52"
+    } else if cfg!(feature = "testnet-test") {
+        "84a82bb39c83989d5dc07e1310281923d2544dc2"
+    } else {
+        panic!("This test requires mainnet-test or testnet-test feature enabled.")
+    };
+    let target_address = "1111111122222222333333334444444455555555";
+    let amount: u64 = 1_000_000;
+    let amount_bytes = amount.to_le_bytes();
+    let payload = hex::decode(format!(
+        "000000{}{}{}",
+        hex::encode(amount_bytes),
+        target_address,
+        eth_custodian_address
+    ))
+    .unwrap();
+
+    let tx = contract.call_method_with_args("echo", &[ethabi::Token::Bytes(payload)], nonce.into());
+    let sender = test_utils::address_from_secret_key(&signer.secret_key);
+    let view_call_args = test_utils::as_view_call(tx, sender);
+    let input = view_call_args.try_to_vec().unwrap();
+
+    let (_outcome, maybe_error) = runner.one_shot().call("view", "viewer", input);
+    let error_message = format!("{:?}", maybe_error);
+    assert!(error_message.contains("ERR_ILLEGAL_RETURN"));
+}
diff --git a/engine-tests/src/tests/mod.rs b/engine-tests/src/tests/mod.rs
@@ -6,6 +6,7 @@ mod eip1559;
 mod erc20;
 mod erc20_connector;
 mod eth_connector;
+mod ghsa_3p69_m8gg_fwmf;
 #[cfg(feature = "meta-call")]
 mod meta_parsing;
 mod multisender;

diff --git a/engine-tests/src/tests/one_inch.rs b/engine-tests/src/tests/one_inch.rs
@@ -39,7 +39,7 @@ fn test_1inch_liquidity_protocol() {
     let (result, profile, pool) =
         helper.create_pool(&pool_factory, token_a.0.address, token_b.0.address);
     assert!(result.gas_used >= 4_500_000); // more than 4.5M EVM gas used
-    assert_gas_bound(profile.all_gas(), 22);
+    assert_gas_bound(profile.all_gas(), 21);
 
     // Approve giving ERC-20 tokens to the pool
     helper.approve_erc20_tokens(&token_a, pool.address());
@@ -58,7 +58,7 @@ fn test_1inch_liquidity_protocol() {
         },
     );
     assert!(result.gas_used >= 302_000); // more than 302k EVM gas used
-    assert_gas_bound(profile.all_gas(), 25);
+    assert_gas_bound(profile.all_gas(), 24);
 
     // Same here
     helper.runner.context.block_timestamp += 10_000_000 * 1_000_000_000;
@@ -73,7 +73,7 @@ fn test_1inch_liquidity_protocol() {
         },
     );
     assert!(result.gas_used >= 210_000); // more than 210k EVM gas used
-    assert_gas_bound(profile.all_gas(), 27);
+    assert_gas_bound(profile.all_gas(), 25);
 
     let (result, profile) = helper.pool_withdraw(
         &pool,
@@ -84,7 +84,7 @@ fn test_1inch_liquidity_protocol() {
         },
     );
     assert!(result.gas_used >= 150_000); // more than 150k EVM gas used
-    assert_gas_bound(profile.all_gas(), 23);
+    assert_gas_bound(profile.all_gas(), 21);
 }
 
 #[test]

diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs
@@ -27,7 +27,7 @@ fn repro_GdASJ3KESs() {
         block_timestamp: 1645717564644206730,
         input_path: "src/tests/res/input_GdASJ3KESs.hex",
         evm_gas_used: 706713,
-        near_gas_used: 138,
+        near_gas_used: 133,
     });
 }
 
@@ -52,7 +52,7 @@ fn repro_8ru7VEA() {
         block_timestamp: 1648829935343349589,
         input_path: "src/tests/res/input_8ru7VEA.hex",
         evm_gas_used: 1732181,
-        near_gas_used: 250,
+        near_gas_used: 242,
     });
 }
 
@@ -72,7 +72,7 @@ fn repro_FRcorNv() {
         block_timestamp: 1650960438774745116,
         input_path: "src/tests/res/input_FRcorNv.hex",
         evm_gas_used: 1239721,
-        near_gas_used: 203,
+        near_gas_used: 198,
     });
 }
 
@@ -107,7 +107,7 @@ fn repro_D98vwmi() {
         block_timestamp: 1651753443421003245,
         input_path: "src/tests/res/input_D98vwmi.hex",
         evm_gas_used: 1_035_348,
-        near_gas_used: 205,
+        near_gas_used: 199,
     });
 }
 

diff --git a/engine-tests/src/tests/res/echo.sol b/engine-tests/src/tests/res/echo.sol
@@ -0,0 +1,17 @@
+ // SPDX-License-Identifier: GPL-3.0
+
+ pragma solidity >=0.7.0 <0.9.0;
+
+ contract Echo {
+
+     function echo(bytes memory payload) public pure {
+         assembly {
+             let pos := mload(0x40)
+
+             mstore(pos, mload(add(payload, 0x20)))
+             mstore(add(pos, 0x20), mload(add(payload, 0x40)))
+
+             return(pos, 51)
+         }
+     }
+ }
diff --git a/engine-tests/src/tests/res/self_destructor.sol b/engine-tests/src/tests/res/self_destructor.sol
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+contract SelfDestruct {
+   constructor() payable {}
+
+   function destruct(address benefactor) payable external {
+      selfdestruct(payable(benefactor));
+   }
+
+}