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

ERC20Internals._transfer updates the same storage slot twice when from == to

Author Revealed upon completion

Any token holder can mint arbitrary tokens for themselves by repeatedly calling transfer(msg.sender, value) or transferFrom(holder, holder, value). The sum of balances no longer matches _totalSupply, and any DeFi protocol that trusts this token’s balances can be drained.Description

  • Normal behavior

    In a standard ERC20 implementation, a transfer with from == to (self-transfer) is allowed and results in no balance change. The Transfer event is emitted, but the sender’s balance remains the same and total supply is unaffected:


    • balance[from] stays the same.

    • balance[to] stays the same (same account).

    • totalSupply stays the same.


  • Issue

    In this implementation, _transfer calculates fromSlot and toSlot separately but does not handle the case where they are identical (from == to). The function:


    1. Loads fromAmount and toAmount from the same slot.

    2. Writes fromAmount - value to that slot.

    3. Then writes toAmount + value (using the original value) to the same slot.


    When from == to, this results in the final balance being fromAmount + value, effectively minting value tokens for the caller on every self-transfer.

function _transfer(address from, address to, uint256 value) internal returns (bool success) {
assembly ("memory-safe") {
if iszero(from) {
mstore(0x00, shl(224, 0x96c6fd1e))
mstore(add(0x00, 4), 0x00)
revert(0x00, 0x24)
}
​
if iszero(to) {
mstore(0x00, shl(224, 0xec442f05))
mstore(add(0x00, 4), 0x00)
revert(0x00, 0x24)
}
​
let ptr := mload(0x40)
let baseSlot := _balances.slot
​
// slot for `from`
mstore(ptr, from)
mstore(add(ptr, 0x20), baseSlot)
let fromSlot := keccak256(ptr, 0x40)
let fromAmount := sload(fromSlot)
​
//@> mstore(ptr, to)
//@> mstore(add(ptr, 0x20), baseSlot)
//@> let toSlot := keccak256(ptr, 0x40)
//@> let toAmount := sload(toSlot)
​
if lt(fromAmount, value) {
mstore(0x00, shl(224, 0xe450d38c))
mstore(add(0x00, 4), from)
mstore(add(0x00, 0x24), fromAmount)
mstore(add(0x00, 0x44), value)
revert(0x00, 0x64)
}
​
// When fromSlot == toSlot:
// 1) write fromAmount - value
// 2) overwrite with toAmount + value (using the original toAmount == fromAmount)
//@> sstore(fromSlot, sub(fromAmount, value))
//@> sstore(toSlot, add(toAmount, value))
​
success := 1
mstore(ptr, value)
log3(ptr, 0x20, 0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef, from, to)
}
}

Risk

Likelihood:

  • Whenever a holder calls transfer(msg.sender, value) with value > 0, their balance increases by value instead of remaining unchanged.

Impact:

  • Any holder can mint arbitrary tokens by looping self-transfers, then deposit these inflated balances into DeFi protocols that integrate this token, draining their pools or breaking assumptions around value.

  • The invariant sum(balances) == totalSupply() no longer holds, because _totalSupply is unchanged while balances grow, which breaks accounting, risk models, and any system that relies on total supply to reason about caps or shares.


Proof of Concept:

You can paste the following test into Token.t.sol to demonstrate the bug and the broken totalSupply invariant.

function test_selfTransferMintsTokens() public {
address user = makeAddr("user");
​
// Mint initial balance
uint256 initial = 100e18;
token.mint(user, initial);
​
assertEq(token.balanceOf(user), initial);
assertEq(token.totalSupply(), initial);
​
// user self-transfers 10 tokens
uint256 value = 10e18;
vm.prank(user);
token.transfer(user, value);
​
// Balance increased instead of remaining the same
uint256 finalBalance = token.balanceOf(user);
uint256 finalSupply = token.totalSupply();
​
// Shows the invariant break clearly
assertEq(finalSupply, initial, "totalSupply should remain unchanged by transfer");
assertEq(finalBalance, initial + value, "self-transfer incorrectly mints tokens");
assertGt(finalBalance, finalSupply, "balance is now greater than totalSupply");
}

Recommended Mitigation

Ensure that self-transfers do not change balances. The simplest fix is to detect when from and to maps to the same slot and avoid applying both subtract and add operations to the same storage slot.

function _transfer(address from, address to, uint256 value) internal returns (bool success) {
assembly ("memory-safe") {
...
mstore(ptr, from)
mstore(add(ptr, 0x20), baseSlot)
let fromSlot := keccak256(ptr, 0x40)
let fromAmount := sload(fromSlot)
​
mstore(ptr, to)
mstore(add(ptr, 0x20), baseSlot)
let toSlot := keccak256(ptr, 0x40)
let toAmount := sload(toSlot)
​
if lt(fromAmount, value) {
...
}
​
- sstore(fromSlot, sub(fromAmount, value))
- sstore(toSlot, add(toAmount, value))
+ // Avoid double-writing the same slot for self-transfers
+ if eq(fromSlot, toSlot) {
+ // Self-transfer: balances must remain unchanged.
+ // Optionally, only emit the Transfer event after checking balance.
+ } else {
+ sstore(fromSlot, sub(fromAmount, value))
+ sstore(toSlot, add(toAmount, value))
+ }
​
success := 1
...
}
}

Support

FAQs

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

Give us feedback!