ERC20Internals.sol _burn() does not check for underflow + burning more tokens than ERC20.sol totalSupply() will result in underflow increasing the totalSupply()
Description
-
ERC20Internals.sol **_burn()**should check and revert on underflow.
-
_burn() does not check for underflow. Burning more than ERC20.sol totalSupply() tokens will result in underflow increasing the ERC20.sol totalSupply().
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)
sstore(accountBalanceSlot, sub(accountBalance, value))
}
}
Risk
Likelihood:
Impact:
Proof of Concept
Place at the top of test/Token.t.sol:
import {console} from "forge-std/Test.sol";
Place in contract test/Token.t.sol Tokentest:
function test_burnUnderflow() public {
uint256 amount = 1;
address account = makeAddr("account");
token.mint(account, amount);
uint256 balance = token.balanceOf(account);
assertEq(balance, amount);
uint256 totalSupply = token.totalSupply();
assertEq(totalSupply, amount);
console.log("balance");
console.log(balance);
console.log("token.totalSupply()");
console.log(totalSupply);
uint256 amountToBurn = amount + 1;
token.burn(account, amountToBurn);
balance = token.balanceOf(account);
totalSupply = token.totalSupply();
assertEq(balance, type(uint256).max);
assertEq(totalSupply, type(uint256).max);
console.log("balance after");
console.log(balance);
console.log("token.totalSupply() after");
console.log(totalSupply);
}
Lines 2 to 16: Mint the 1 token.
Line 18 to 19: Burn 1 more than the totalSupply(). In this case it's 1 + 1 = 2 tokens.
Lines 20 to 31: check that underflow indeed occurs.
Run with:
forge test -vvv --match-test test_burnUnderflow
Sample output:
Ran 1 test for test/Token.t.sol:TokenTest
[PASS] test_burnUnderflow() (gas: 73131)
Logs:
balance
1
token.totalSupply()
1
balance after
115792089237316195423570985008687907853269984665640564039457584007913129639935
token.totalSupply() after
115792089237316195423570985008687907853269984665640564039457584007913129639935
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.17ms (801.25µs CPU time)
Ran 1 test suite in 25.92ms (2.17ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
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)
+
+ // test for underflow
+ let predictedSupply := sub(supply, value)
+ if gt(predictedSupply, supply) {
+ revert(0, 0)
+ }
+
sstore(supplySlot, sub(supply, value))
mstore(ptr, account)
mstore(add(ptr, 0x20), balanceSlot)
let accountBalanceSlot := keccak256(ptr, 0x40)
let accountBalance := sload(accountBalanceSlot)
sstore(accountBalanceSlot, sub(accountBalance, value))
}
}
