From 421a1b729bcfc0f4f57d949d4ee5b4b372d21051 Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 1 Mar 2023 07:09:00 +0100 Subject: [PATCH 1/6] fix: ensure `sender_id` has a balance is greater than amount --- engine/src/fungible_token.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/engine/src/fungible_token.rs b/engine/src/fungible_token.rs index 24f98ecc0..8bfedc944 100644 --- a/engine/src/fungible_token.rs +++ b/engine/src/fungible_token.rs @@ -253,6 +253,13 @@ impl FungibleTokenOps { // Special case for Aurora transfer itself - we shouldn't transfer if sender_id != receiver_id { self.internal_transfer_eth_on_near(&sender_id, &receiver_id, amount, memo)?; + } else { + let balance = self + .get_account_eth_balance(&sender_id) + .unwrap_or(ZERO_NEP141_WEI); + if amount > balance { + return Err(error::TransferError::InsufficientFunds); + } } let args = serde_json::to_vec(&NEP141FtOnTransferArgs { sender_id: sender_id.clone(), From 57a5ebdb3f03cb8de9af3ee57276f7ceca382b6d Mon Sep 17 00:00:00 2001 From: ahmed Date: Thu, 9 Mar 2023 14:36:15 +0100 Subject: [PATCH 2/6] fix: clippy errors --- engine/src/fungible_token.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/engine/src/fungible_token.rs b/engine/src/fungible_token.rs index 04495b884..e3756a1f7 100644 --- a/engine/src/fungible_token.rs +++ b/engine/src/fungible_token.rs @@ -254,13 +254,12 @@ impl FungibleTokenOps { // Special case for Aurora transfer itself - we shouldn't transfer if sender_id != receiver_id { self.internal_transfer_eth_on_near(&sender_id, &receiver_id, amount, memo)?; - } else { - let balance = self - .get_account_eth_balance(&sender_id) - .unwrap_or(ZERO_NEP141_WEI); - if amount > balance { - return Err(error::TransferError::InsufficientFunds); - } + } + let balance = self + .get_account_eth_balance(&sender_id) + .unwrap_or(ZERO_NEP141_WEI); + if amount > balance { + return Err(error::TransferError::InsufficientFunds); } let args = serde_json::to_vec(&NEP141FtOnTransferArgs { sender_id: sender_id.clone(), From 2e502e35d813047f555650f7380cd225dc6ecc25 Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 29 Mar 2023 14:02:05 +0200 Subject: [PATCH 3/6] fix: prevent setting an arbitrary value for `amount` --- engine/src/fungible_token.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/engine/src/fungible_token.rs b/engine/src/fungible_token.rs index e3756a1f7..1f63006ea 100644 --- a/engine/src/fungible_token.rs +++ b/engine/src/fungible_token.rs @@ -251,16 +251,17 @@ impl FungibleTokenOps { current_account_id: AccountId, prepaid_gas: NearGas, ) -> Result { - // Special case for Aurora transfer itself - we shouldn't transfer - if sender_id != receiver_id { - self.internal_transfer_eth_on_near(&sender_id, &receiver_id, amount, memo)?; - } + // check balance to prevent setting an arbitrary value for `amount` for (receiver_id == receiver_id). let balance = self .get_account_eth_balance(&sender_id) .unwrap_or(ZERO_NEP141_WEI); if amount > balance { return Err(error::TransferError::InsufficientFunds); } + // Special case for Aurora transfer itself - we shouldn't transfer + if sender_id != receiver_id { + self.internal_transfer_eth_on_near(&sender_id, &receiver_id, amount, memo)?; + } let args = serde_json::to_vec(&NEP141FtOnTransferArgs { sender_id: sender_id.clone(), amount: Balance::new(amount.as_u128()), From 7e401bd73bab0c35b00536851e36e576b52815fd Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 29 Mar 2023 17:51:23 +0200 Subject: [PATCH 4/6] adding test cases --- engine-tests/src/tests/eth_connector.rs | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/engine-tests/src/tests/eth_connector.rs b/engine-tests/src/tests/eth_connector.rs index 8f3dbefae..cfd9baa9a 100644 --- a/engine-tests/src/tests/eth_connector.rs +++ b/engine-tests/src/tests/eth_connector.rs @@ -501,6 +501,46 @@ fn test_ft_transfer_call_without_message() { let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC); assert_eq!(balance, DEPOSITED_FEE); + // should revert with `not enough balance` error when sending arbitray amount while sender_id = receiver_id + let transfer_amount = 1000000000; + let res = recipient_account.call( + CONTRACT_ACC.parse().unwrap(), + "ft_transfer_call", + json!({ + "receiver_id": recipient_account.signer.account_id.to_string(), + "amount": transfer_amount.to_string(), + "msg": "", + }) + .to_string() + .as_bytes(), + DEFAULT_GAS, + 1, + ); + + assert_execution_status_failure( + res.outcome().clone().status, + "ExecutionError(\"Smart contract panicked: ERR_NOT_ENOUGH_BALANCE\")", + "Expected failure in `ft_transfer_call` call, but call succeeded", + ); + + // should not revert with `not enough balance` error when sending arbitray amount while sender_id = receiver_id with amount < balance + let transfer_amount = 1; + let res = recipient_account.call( + CONTRACT_ACC.parse().unwrap(), + "ft_transfer_call", + json!({ + "receiver_id": recipient_account.signer.account_id.to_string(), + "amount": transfer_amount.to_string(), + "msg": "", + }) + .to_string() + .as_bytes(), + DEFAULT_GAS, + 1, + ); + + res.assert_success(); + // Sending to random account should not change balances let transfer_amount = 22; let res = recipient_account.call( From b637b51a215fa5918aa93325eaf7fe2b8611d1c3 Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Wed, 29 Mar 2023 20:22:43 +0200 Subject: [PATCH 5/6] Update engine-tests/src/tests/eth_connector.rs Co-authored-by: Oleksandr Anyshchenko --- engine-tests/src/tests/eth_connector.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-tests/src/tests/eth_connector.rs b/engine-tests/src/tests/eth_connector.rs index cfd9baa9a..ece55300a 100644 --- a/engine-tests/src/tests/eth_connector.rs +++ b/engine-tests/src/tests/eth_connector.rs @@ -501,7 +501,7 @@ fn test_ft_transfer_call_without_message() { let balance = get_eth_on_near_balance(&master_account, CONTRACT_ACC, CONTRACT_ACC); assert_eq!(balance, DEPOSITED_FEE); - // should revert with `not enough balance` error when sending arbitray amount while sender_id = receiver_id + // should revert with `not enough balance` error when sending arbitrary amount while sender_id == receiver_id let transfer_amount = 1000000000; let res = recipient_account.call( CONTRACT_ACC.parse().unwrap(), From 44c95350822322a08d9c1de6004675a8396e7aa9 Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Wed, 29 Mar 2023 20:22:51 +0200 Subject: [PATCH 6/6] Update engine-tests/src/tests/eth_connector.rs Co-authored-by: Oleksandr Anyshchenko --- engine-tests/src/tests/eth_connector.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-tests/src/tests/eth_connector.rs b/engine-tests/src/tests/eth_connector.rs index ece55300a..f6283423a 100644 --- a/engine-tests/src/tests/eth_connector.rs +++ b/engine-tests/src/tests/eth_connector.rs @@ -523,7 +523,7 @@ fn test_ft_transfer_call_without_message() { "Expected failure in `ft_transfer_call` call, but call succeeded", ); - // should not revert with `not enough balance` error when sending arbitray amount while sender_id = receiver_id with amount < balance + // should not revert with `not enough balance` error when sending arbitrary amount while sender_id == receiver_id with amount < balance let transfer_amount = 1; let res = recipient_account.call( CONTRACT_ACC.parse().unwrap(),