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

0 stars 0 forks source link

ERC20 TRANSFER AND TRANSFERFROM ARE NOT CHECKING FOR THE ZERO ADDRESS OF `to` AND `from`, DURING EXECUTION #258

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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

ERC20 standard tokens check for the != address(0) condition in the transfer and transferFrom functions for the to address and from address in order to make sure funds are not transfered to zero address and balance is not updated in the balanceOf mapping for the zero address. But in ERC20.sol contract of this project the zero address check is not performed for the to and from address inside the transfer and transferFrom functions which will enable funds transfer to zero address and balanceOf mapping being updated for zero address.

Proof of Concept

But in the ERC20.sol contract of this project, the to and from address is not checked for the zero address inside the transfer and transferFrom functions. This could lead to zero address being updated with token balance in the event of address(0) is passed in as an input parameter to either to address or from address.

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 of the ERC20.sol add the zero address check (!= address(0)) for the to address as follows:

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

  require(to != address(0), "ERC20: transfer to the zero address");

  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 of the ERC20.sol add the zero address check (!= address(0)) for the to and from addresses as follows:

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

  require(from != address(0), "ERC20: transfer from the zero address");
  require(to != address(0), "ERC20: transfer to the zero address");

  uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.

  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;
}
c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid