Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/ERC20.sol#L75-L87 https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/ERC20.sol#L90-L106

Vulnerability details

Impact

transfer and transferFrom functions do not check for the available erc20 token balance of the from address and the msg.sender respectively against the requested transfer amount. Hence if the balanceOf[from] and balanceOf[msg.sender] in the transferFrom and transfer functions is less than the requested transfer amount of uint256 amount, it could trigger underflow since balanceOf[from] - amount will be negative. Same applies for the balanceOf[msg.sender] - amount as well. Due to this underflow condition the erroneous values will be returned.

Proof of Concept

There is no check to confirm balanceOf[from] >= amount and balanceOf[msg.sender] >= amount inside the transferFrom and transfer functions respectively. This could lead to underflow condition and will result in erroneous values returned as the result.

https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/ERC20.sol#L75-L87

https://github.com/code-423n4/2023-01-numoen/blob/main/src/core/ERC20.sol#L90-L106

Tools Used

Manual and VS Code

Recommended Mitigation Steps

Inside the transfer function check for the balanceOf[msg.sender] >= amount condition as follows:

function transfer(address to, uint256 amount) external virtual returns (bool) {

  require( balanceOf[msg.sender] >= amount, "ERC20: transfer amount exceeds balance");

  balanceOf[msg.sender] -= amount;

  // Cannot overflow because the sum of all user
 // balances can't exceed the max uint256 value.
  unchecked {
    balanceOf[to] += amount;
  }

  emit Transfer(msg.sender, to, amount);

  return true;

}

inside the transferFrom function check for the balanceOf[from] >= amount condition as follows:

function transferFrom(address from, address to, uint256 amount) external virtual returns (bool) {
  uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.

  require(allowed >= amount, "ERC20: transfer amount exceeds allowance");

  if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - amount;

  balanceOf[from] -= amount;

  // Cannot overflow because the sum of all user
  // balances can't exceed the max uint256 value.
  unchecked {
    balanceOf[to] += amount;
  }

  emit Transfer(from, to, amount);

  return true;
}

Above mitigation will check for the underflow condition and will revert the entire transaction in the event of an underflow. Hence there won't be any erroneous values returned due to underflow condition.

code-423n4 / 2023-01-numoen-findings