code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

Use unchecked{} in ERC20 to save gas without risk #3

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

t11s

Vulnerability details

Impact

Since Wildcredit's ERC20 implementation uses the Solidity 0.8 compiler, all math operations are checked against over/underflow by default. However, some of these checks are unnecessary due to pre-checks. Wildcredit can selectively disable these over/underflow checks using the unchecked{} construct.

Proof of Concept

There are 3 lines where use of checked math is unnecessary:

  1. https://github.com/code-423n4/2021-09-wildcredit/blob/edfc97a8b75bb290a7f7f61cda62d586ed42c73e/contracts/external/ERC20.sol#L54

The sum of all balances in the ERC20 will never exceed type(uint256).max, as totalSupply is safely incremented in _mint when creating new tokens. If _mint was ever called with a value that would increase the sum of all balances beyond type(uint256).max, totalSupply would overflow and Solidity's checked math would cause a revert.

Hence, the balanceOf[_recipient] += _amount; can be wrapped in unchecked to prevent an unnecessary overflow check on the recipient's balance, as it cannot ever overflow.

  1. https://github.com/code-423n4/2021-09-wildcredit/blob/edfc97a8b75bb290a7f7f61cda62d586ed42c73e/contracts/external/ERC20.sol#L62

Same as #1. totalSupply would overflow and revert first if the recipient would receive too many tokens.

  1. https://github.com/code-423n4/2021-09-wildcredit/blob/edfc97a8b75bb290a7f7f61cda62d586ed42c73e/contracts/external/ERC20.sol#L70

Inverse of #2. A user's balance can never exceed totalSupply, hence decrementing of the totalSupply will never cause an underflow.

Recommended Mitigation Steps

Wrap the 3 lines mentioned above in unchecked {}