Assignment: Audit a Smart Contract

Objective:

The purpose of this assignment is to provide students with hands-on experience in auditing and implementing smart contracts. Students will be given a basic Solana smart contract written in the Rust programming language and are expected to identify any bugs or security risks and provide a report of their findings, as well as a new implementation of the smart contract that fixes any identified issues.

Starter Code

use solana_sdk::{
    account::Account,
    entrypoint::{entrypoint, Entrypoint},
    program_error::ProgramError,
    pubkey::Pubkey,
};

struct Token {
    owner: Pubkey,
    balance: u64,
}

impl Token {
    fn new(owner: Pubkey) -> Self {
        Self { owner, balance: 0 }
    }

    fn deposit(&mut self, amount: u64) {
        self.balance += amount;
    }

    fn transfer(&mut self, to: Pubkey, amount: u64) -> Result<(), ProgramError> {
        if self.balance < amount {
            return Err(ProgramError::InsufficientFunds);
        }
        if self.owner == to {
            return Err(ProgramError::InvalidAccount);
        }
        self.balance -= amount;
        Ok(())
    }
}

#[entrypoint] 
// Don't worry about this
pub fn process_instruction(
    program_id: &Pubkey,
    accounts: &[Account],
    instruction_data: &[u8],
) -> Result<(), ProgramError> {
    let account_metas = &accounts[0..2];
    let source_account = &account_metas[0];
    let destination_account = &account_metas[1];
    let source_data = &source_account.data;
    let destination_data = &destination_account.data;
    let source_token = Token::deserialize(source_data).unwrap();
    let mut destination_token = Token::deserialize(destination_data).unwrap();

    match instruction_data[0] {
        0 => source_token.transfer(destination_account.key, 1)?,
        1 => destination_token.deposit(1),
        2 => destination_token.transfer(source_account.key, 1)?,
        _ => return Err(ProgramError::InvalidInstructionData),
    }

    source_account.data = source_token.serialize();
    destination_account.data = destination_token.serialize();
    Ok(())
}

Instructions:

1. Familiarize yourself with the given Solana smart contract code.

2. Conduct a thorough code review and identify any bugs or security risks in the smart contract.

3. Write a report detailing all of the bugs and security risks you have identified and the risks associated with them.

4. Provide a new implementation of the smart contract that addresses the issues you identified in your report.

Report Requirements:

1. A clear and concise description of the bugs and security risks you have identified.

2. A discussion of the risks associated with each issue.

3. A detailed explanation of how each issue can be fixed.

New Implementation Requirements:

1. The new implementation must address all of the issues identified in the report.

2. The new implementation must be clearly and concisely written.

3. The new implementation must be well-documented with comments explaining the purpose of each section of code.

Submission:

1. Submit your report in a pdf file format.

2. Submit the new implementation in a .rs file format.

3. Submit both files by pushing to the GitHub repo

Hint (errors)

1. Unchecked Deserialization: The contract calls Token::deserialize without checking if the deserialization is successful. This could result in a panic or other unexpected behavior if the input data is invalid.

2. Insufficient Error Handling: The contract does not check for errors when performing the transfer operation, which could result in incorrect balances if an error occurs.

3. Lack of Authorization: The contract does not check who is calling the process_instruction function, which could allow any account to transfer tokens on behalf of the token owner.

4. Integer Overflow: The contract does not check for integer overflow when adding to the balance, which could result in unexpected behavior if the balance becomes too large.

5. Lack of Reentrancy Protection: The contract does not have any protection against reentrancy attacks, where an attacker could call the contract again while it is in the middle of processing an instruction, leading to unexpected behavior.

6. Lack of Event Emission: The contract does not emit events when a transfer is made, which could make it difficult to track the history of transfers and view the state of the token.

7. No Gas Metering: The contract does not implement gas metering, which is important for ensuring that the contract is usable on a large scale and does not consume an excessive amount of resources.

Solution (code)

This version of the contract implements proper error handling using the Result type, checks for unauthorized access to the process_instruction function, and adds protection against integer overflow.

use solana_sdk::{
    account::Account,
    entrypoint::{entrypoint, Entrypoint},
    program_error::ProgramError,
    pubkey::Pubkey,
};

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Token {
    pub owner: SolanaAccountId,
    pub balance: u64,
}

impl Token {
    pub fn new(owner: SolanaAccountId) -> Self {
        Self {
            owner,
            balance: 0,
        }
    }

    pub fn transfer(&mut self, to: &SolanaAccountId, amount: u64) -> Result<(), &'static str> {
        if amount > self.balance {
            return Err("Insufficient funds");
        }
        self.balance -= amount;
        Ok(())
    }

    pub fn deposit(&mut self, amount: u64) {
        self.balance = self.balance.checked_add(amount).expect("Integer overflow");
    }

    pub fn serialize(&self) -> Vec<u8> {
        bincode::serialize(self).expect("Serialization failed")
    }

    pub fn deserialize(input: &[u8]) -> Result<Self, bincode::Error> {
        bincode::deserialize(input)
    }
}

#[solana_program]
pub fn process_instruction(
    program_id: &SolanaAccountId,
    accounts: &[SolanaAccount],
    instruction_data: &[u8],
) -> Result<(), ProgramError> {
    let from = &instruction_data[0..32];
    let to = &instruction_data[32..64];
    let amount = u64::from_le_bytes(instruction_data[64..72].try_into().unwrap());
    let caller_id = solana_sdk::pubkey::new_rand();

    if program_id != &caller_id {
        return Err(ProgramError::Unauthorized);
    }

    let from_account = &accounts[0];
    let to_account = &accounts[1];

    let mut from_token = Token::deserialize(&from_account.data)
        .map_err(|_| ProgramError::InvalidAccountData)?;

    let mut to_token = Token::deserialize(&to_account.data)
        .map_err(|_| ProgramError::InvalidAccountData)?;

    if from_token.owner != from {
        return Err(ProgramError::Unauthorized);
    }

    from_token.transfer(to, amount)?;
    to_token.deposit(amount);

    solana_sdk::pubkey::check_id(from, &from_token.owner)?;
    solana_sdk::pubkey::check_id(to, &to_token.owner)?;

    from_account.data = from_token.serialize();
    to_account.data = to_token.serialize();

    Ok(())
}