Smart Contract Audit

Security
1

Hello I have a solidity code to audit like this

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

// Allow to split the balance through complex rules

interface Split{

function getAddressAndAmountToSplit() view external returns(address, uint);

}

// MyBank contract

// This contract allows anyone to store any ERC20 tokens

contract MyBank {

// (token => user => amount)

mapping (address => mapping(address => uint)) public userBalance;

// (address => Split contract)

mapping (address => Split) splits;

// Deposit ERC20 tokens to the contracts

// The user must approve the bank before calling addToBalance

function addToBalance(IERC20 token, uint amount) external

{

token.transferFrom(msg.sender, address(this), amount);

userBalance[address(token)][msg.sender] += amount;

}

// Withdraw part of the balance

function withdrawBalance(IERC20 token) external

{

token.transfer(msg.sender, userBalance[address(token)][msg.sender]);

userBalance[address(token)][msg.sender] = 0;

}

// Allow to register a split contract

function registerSplit(Split split) external{

splits[msg.sender] = split;

}

// Split the balance into two accounts

// The usage of a Split contract allows to create complex split strategies

function splitBalance(IERC20 token) external

{

Split split = splits[msg.sender];

require(split != Split(address(0x0)));

uint balance = userBalance[address(token)][msg.sender];

(address dest, uint amount) = Split(split).getAddressAndAmountToSplit();

userBalance[address(token)][dest] = amount;

userBalance[address(token)][msg.sender] = balance - amount;

}
}

What I found.

  1. function withdrawBalance(IERC20 token) external possible reentrancy attack, because we check balance in the end
  2. function splitBalance(IERC20 token) external - vulnerable business logic, because if amount is greater than balance we get negative value and possible integer overflow

If you have any idea of possible vulnerabilities of code above, please feel free to provide any further assistance

1 Like
2

you should be checking the bool returned by the transfer and transferFrom calls.

the only call to an external contract in withdrawBalance is the transfer call. it's vulnerable to a reentrancy attack if your contract handles an arbitrary, user controlled token that can perform the attack on that call.

add a lock if that is the case:

bool internal _locked = false;
modifier lock {
    require(!_locked, "locked");
    _locked = true;
    _;
    _locked = false;
}
1 Like
3

I’ll advice you get an expert service.

Fortunately HashEx is offering FREE audits for smart contract.

You can register and get a free audit from HashEx... they’ve been in blockchain security for over 4 years, audited over 500 smart contracts and save real projects billions of $$

Save money while getting the best!