Token-0x
First Flight #54
Beginner FriendlyDeFi
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: medium

Individual user balances are unprotected from underflow in `ERC20Internals::_burn`

Author Revealed upon completion

Description

Similar to transfer, an ERC20 burn operation must prevent user balances from going below 0, since subtracting more than the current balance will underflow and wrap to type(uint256).max.

In ERC20Internals::_burn, the contract subtracts value from the user’s balance in inline assembly without checking that value <= accountBalance. Because inline assembly uses unchecked arithmetic, the balance can underflow and wrap to type(uint256).max.

function _burn(address account, uint256 value) internal {
assembly ("memory-safe") {
if iszero(account) {
mstore(0x00, shl(224, 0x96c6fd1e))
mstore(add(0x00, 4), 0x00)
revert(0x00, 0x24)
}
​
let ptr := mload(0x40)
let balanceSlot := _balances.slot
let supplySlot := _totalSupply.slot
​
let supply := sload(supplySlot)
sstore(supplySlot, sub(supply, value))
​
mstore(ptr, account)
mstore(add(ptr, 0x20), balanceSlot)
​
// @audit high - no underflow check on account balance
let accountBalanceSlot := keccak256(ptr, 0x40)
let accountBalance := sload(accountBalanceSlot)
@> sstore(accountBalanceSlot, sub(accountBalance, value))
}
}

Risk

Likelihood:

Underflow occurs whenever _burn is called with an amount greater than the user’s balance. Because _burn does not enforce this check, any misuse by an inheriting contract or future extension can immediately trigger the issue.

Impact:

This breaks the ERC20 accounting, causing incorrect pricing and share accounting. An underflow in a user's balance will inflate the token balance to type(uint256).max, which could enable protocol This breaks ERC20 accounting, causing incorrect pricing, share accounting, and governance weight. An underflow in a user’s balance inflates it to type(uint256).max, which can enable manipulation of any protocol that accepts the token.

Proof of Concept

Add the following test to the test suite in test/Token.t.sol.

function testNoUnderflowCheckForIndividualBalanceInBurn() public {
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
// just in case the _burn function actually checks totalSupply underflow
token.mint(user1, 1);
token.burn(user2, 1);
// underflowed after 0 - 1 = type(uint256).max
assert(token.balanceOf(user2) == type(uint256).max);
}

This test shows that the token balance of user2 underflowed to type(uint256).max after a burn when user2 had a balance of 0.

Recommended Mitigation

function _burn(address account, uint256 value) internal {
assembly ("memory-safe") {
if iszero(account) {
mstore(0x00, shl(224, 0x96c6fd1e))
mstore(add(0x00, 4), 0x00)
revert(0x00, 0x24)
}
​
let ptr := mload(0x40)
let balanceSlot := _balances.slot
let supplySlot := _totalSupply.slot
​
let supply := sload(supplySlot)
sstore(supplySlot, sub(supply, value))
​
mstore(ptr, account)
mstore(add(ptr, 0x20), balanceSlot)
​
let accountBalanceSlot := keccak256(ptr, 0x40)
let accountBalance := sload(accountBalanceSlot)
+ if lt(accountBalance, value) {
+ revert(0,0)
+ }
sstore(accountBalanceSlot, sub(accountBalance, value))
}
}

Make sure to replace revert(0, 0) with a custom error to align with the rest of the error handling in the contract.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!