Bladeren bron

Avoid overflow in transfer (#1485)

`address.transfer` and `address.send` utilize `void sol_transfer` from
`solana.c` under the hood, which was saving the overflowed valued to the
account balance. This PR fixes such an issue.

Signed-off-by: Lucas Steuernagel <lucas.tnagel@gmail.com>
Lucas Steuernagel 2 jaren geleden
bovenliggende
commit
de11ed32ae
3 gewijzigde bestanden met toevoegingen van 59 en 8 verwijderingen
  1. 4 5
      docs/language/managing_values.rst
  2. 8 2
      stdlib/solana.c
  3. 47 1
      tests/solana_tests/balance.rs

+ 4 - 5
docs/language/managing_values.rst

@@ -95,8 +95,7 @@ Here is an example:
     function to be called, use ``address.call{value: 100}("")`` instead.
 
 .. note::
-    On Solana, these functions manage native token assets from a contract's data account when
-    invoked from a contract variable. Normally, funds are not stored in a contract's account, as
-    one can set a separate payer account to pay for all the transactions. In this case, it is more
-    straightforward to use the :ref:`system instruction library <system_instruction_library>` to transfer
-    native tokens between accounts.
+    On Solana, ``send()`` and ``transfer()`` can only transfer native tokens between accounts owned
+    by the contract's program account, since only the account owner can modify its balance.
+    Use the :ref:`system instruction library <system_instruction_library>` to transfer
+    native tokens between accounts owned by Solana's system program.

+ 8 - 2
stdlib/solana.c

@@ -154,17 +154,23 @@ void sol_transfer(uint8_t *to_address, uint64_t lamports, SolParameters *params)
     uint64_t *from = params->ka[0].lamports;
     uint64_t *to = sol_account_lamport(to_address, params);
 
-    if (__builtin_sub_overflow(*from, lamports, from))
+    uint64_t from_balance;
+    uint64_t to_balance;
+
+    if (__builtin_sub_overflow(*from, lamports, &from_balance))
     {
         sol_log("sender does not have enough balance");
         sol_panic();
     }
 
-    if (__builtin_add_overflow(*to, lamports, to))
+    if (__builtin_add_overflow(*to, lamports, &to_balance))
     {
         sol_log("recipient lamports overflows");
         sol_panic();
     }
+
+    *from = from_balance;
+    *to = to_balance;
 }
 
 bool sol_try_transfer(uint8_t *to_address, uint64_t lamports, SolParameters *params)

+ 47 - 1
tests/solana_tests/balance.rs

@@ -313,6 +313,29 @@ fn transfer_fails_not_enough() {
         }])
         .must_fail();
     assert!(res.is_err());
+
+    // Ensure the balance in the account has not overflowed
+    assert_eq!(vm.account_data[&data_account].lamports, 103);
+    assert_eq!(vm.account_data[&new].lamports, 5);
+
+    vm.function("transfer")
+        .arguments(&[
+            BorshToken::Address(new),
+            BorshToken::Uint {
+                width: 64,
+                value: BigInt::from(103u8),
+            },
+        ])
+        .accounts(vec![("dataAccount", data_account)])
+        .remaining_accounts(&[AccountMeta {
+            pubkey: Pubkey(new),
+            is_signer: false,
+            is_writable: true,
+        }])
+        .call();
+
+    assert_eq!(vm.account_data[&data_account].lamports, 0);
+    assert_eq!(vm.account_data[&new].lamports, 108);
 }
 
 #[test]
@@ -329,7 +352,7 @@ fn transfer_fails_overflow() {
     );
 
     let data_account = vm.initialize_data_account();
-    vm.account_data.get_mut(&data_account).unwrap().lamports = 103;
+    vm.account_data.get_mut(&data_account).unwrap().lamports = 104;
 
     vm.function("new")
         .accounts(vec![("dataAccount", data_account)])
@@ -363,6 +386,29 @@ fn transfer_fails_overflow() {
         }])
         .must_fail();
     assert!(res.is_err());
+
+    // Ensure no change in the values
+    assert_eq!(vm.account_data[&new].lamports, u64::MAX - 100);
+    assert_eq!(vm.account_data[&data_account].lamports, 104);
+
+    vm.function("transfer")
+        .arguments(&[
+            BorshToken::FixedBytes(new.to_vec()),
+            BorshToken::Uint {
+                width: 64,
+                value: BigInt::from(100u8),
+            },
+        ])
+        .accounts(vec![("dataAccount", data_account)])
+        .remaining_accounts(&[AccountMeta {
+            pubkey: Pubkey(new),
+            is_writable: false,
+            is_signer: true,
+        }])
+        .call();
+
+    assert_eq!(vm.account_data[&new].lamports, u64::MAX);
+    assert_eq!(vm.account_data[&data_account].lamports, 4);
 }
 
 #[test]