Root + Impact
Description
-
Normal ERC20 behavior: when a user transfers tokens to themselves (from == to), their balance should remain unchanged (net effect 0, only gas is spent).
-
In this implementation, _transfer reads both from and to balances using the same storage slot when from == to, then performs two writes using cached values. The second write overwrites the first, so the final balance becomes fromAmount + value instead of staying fromAmount, effectively minting tokens on 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
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)
}
@> sstore(fromSlot, sub(fromAmount, value))
@> sstore(toSlot, add(toAmount, value))
success := 1
mstore(ptr, value)
log3(ptr, 0x20, 0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef, from, to)
}
}
Risk
Likelihood:
Impact:
-
Impact 1: An attacker can double their balance (or increase it by amount) repeatedly until they own the entire supply or overflow the variable.
-
Impact 2: The token's economic value will drop to zero immediately
Proof of Concept
pragma solidity ^0.8.24;
import {Test} from "forge-std/Test.sol";
import {ERC20} from "../src/ERC20.sol";
contract SelfTransferExploitTest is Test {
ERC20 public token;
address public attacker = address(0x2034);
function setUp() public {
token = new ERC20("SELFFUND", "SLF");
deal(address(token), attacker, 100 ether);
}
function testSelfTransfer() public {
vm.startPrank(attacker);
uint256 initialBalance = token.balanceOf(attacker);
uint256 transferAmount = 10 ether;
token.transfer(attacker, transferAmount);
uint256 finalBalance = token.balanceOf(attacker);
assertEq(finalBalance, initialBalance + transferAmount, "Self-transfer should not mint tokens");
vm.stopPrank();
}
}
Recommended Mitigation
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
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)
}
- sstore(fromSlot, sub(fromAmount, value))
- sstore(toSlot, add(toAmount, value))
+ // Fix: for self-transfers (from == to), do not touch storage.
+ // We still enforce the balance check above and emit the event,
+ // but skipping these writes prevents the infinite mint bug.
+ if iszero(eq(from, to)) {
+ sstore(fromSlot, sub(fromAmount, value))
+ sstore(toSlot, add(toAmount, value))
+ }
success := 1
mstore(ptr, value)
log3(ptr, 0x20, 0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef, from, to)
}
}
