Token-0x

First Flight #54
Beginner FriendlyDeFi
100 EXP
Submission Details
Impact: low
Likelihood: medium

The `ERC20Internals::_spendAllowance` function always subtracts from `allowance` even when it is `type(uint256).max`, breaking infinite allowance semantics and causing unexpected reverts or logic failures in integrations.

Author Revealed upon completion

The ERC20Internals::_spendAllowance function always subtracts from allowance even when it is type(uint256).max, breaking infinite allowance semantics and causing unexpected reverts or logic failures in integrations.

Description

  • In ERC20 implementations like OpenZeppelin, if allowance is set to type(uint256).max, it is treated as infinite and is not reduced when transferFrom is called.

  • In this implementation, _spendAllowance always subtracts the spent value, even when the allowance is at its maximum, removing the semantics of infinite allowance.

function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
assembly ("memory-safe") {
(... )
let allowanceSlot := keccak256(ptr, 0x40)
let currentAllowance := sload(allowanceSlot)
@> // missing check: update only when currentAllowance is lower than type(uint256).max
if lt(currentAllowance, value) {
mstore(0x00, shl(224, 0xfb8f41b2))
mstore(add(0x00, 4), spender)
mstore(add(0x00, 0x24), currentAllowance)
mstore(add(0x00, 0x44), value)
revert(0, 0x64)
}
sstore(allowanceSlot, sub(currentAllowance, value))
}
}

Risk

Likelihood: Medium

  • This manifests whenever an integration uses type(uint256).max assuming infinite allowance.

  • Repeated calls to transferFrom reduce the allowance and eventually cause unexpected reverts.

Impact: Low

  • Does not put funds at risk, but breaks the usual infinite allowance semantics used by many ERC20 integrations.

  • Can cause logic failures in protocols that rely on this standard behavior.

Proof of Concept

This test validates that, even when setting an allowance to type(uint256).max, the _spendAllowance function reduces it after a transferFrom, showing that the implementation does not maintain the infinite allowance semantics typical of OpenZeppelin and may break integrations expecting this behavior.

function test_SpendAllowance_BreaksInfiniteAllowanceBehavior() public {
address alice = makeAddr("alice");
address dex = makeAddr("DEX");
// Give Alice the maximum possible supply
token.mint(alice, type(uint256).max);
// Alice approves DEX with "infinite" allowance
vm.prank(alice);
token.approve(dex, type(uint256).max);
// DEX spends part of the allowance
vm.prank(dex);
token.transferFrom(alice, dex, 563 ether);
// Check that the allowance has decreased,
// showing that it is NOT treated as infinite
assertLt(token.allowance(alice, dex), type(uint256).max);
}
[PASS] test_SpendAllowance_BreaksInfiniteAllowanceBehavior() (gas: 114278)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.55ms (4.99ms CPU time)

Recommended Mitigation

Preserve the standard ERC20 semantics for infinite allowances by updating the allowance only when it is not type(uint256).max, aligning behavior with OpenZeppelin and preventing unwanted consumption of an unlimited allowance.

function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
assembly ("memory-safe") {
(... )
let allowanceSlot := keccak256(ptr, 0x40)
let currentAllowance := sload(allowanceSlot)
- if lt(currentAllowance, value) {
- mstore(0x00, shl(224, 0xfb8f41b2))
- mstore(add(0x00, 4), spender)
- mstore(add(0x00, 0x24), currentAllowance)
- mstore(add(0x00, 0x44), value)
- revert(0, 0x64)
- }
- sstore(allowanceSlot, sub(currentAllowance, value))
+ if lt(currentAllowance, not(0)) {
+ if lt(currentAllowance, value) {
+ mstore(0x00, shl(224, 0xfb8f41b2))
+ mstore(add(0x00, 4), spender)
+ mstore(add(0x00, 0x24), currentAllowance)
+ mstore(add(0x00, 0x44), value)
+ revert(0, 0x64)
+ }
+ sstore(allowanceSlot, sub(currentAllowance, value))
+ }
}
}

Support

FAQs

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

Give us feedback!