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

_spendAllowance has no zero address checks

Author Revealed upon completion

Description

  • In robust ERC‑20 implementations, spending allowance should validate the owner and spender addresses are non‑zero before adjusting allowances. OpenZeppelin enforces this by routing through _approve (which checks zero addresses) ensuring calls with address(0) revert with clear errors.

  • In this codebase, _spendAllowance(owner, spender, value) does not validate owner or spender for zero addresses. It directly computes and updates the nested allowance slot in storage, only checking that currentAllowance >= value. This allows calls with owner == address(0) and/or spender == address(0) to proceed to storage access (or revert with an unrelated InsufficientAllowance), instead of reverting with explicit, standard errors.

// src/helpers/ERC20Internals.sol
function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
assembly ("memory-safe") {
// @> BUG: No zero-address checks for `owner` and `spender`
let ptr := mload(0x40)
let baseSlot := _allowances.slot
mstore(ptr, owner)
mstore(add(ptr, 0x20), baseSlot)
let initialHash := keccak256(ptr, 0x40)
mstore(ptr, spender)
mstore(add(ptr, 0x20), initialHash)
let allowanceSlot := keccak256(ptr, 0x40)
let currentAllowance := sload(allowanceSlot)
if lt(currentAllowance, value) {
// reverts with ERC20InsufficientAllowance
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: High

  • Allowance-spending paths are used constantly by DEXes, routers, and protocols. These paths may unknowingly pass address(0) (config errors, mock addresses, or defensive probing).

  • Since _spendAllowance is called inside transferFrom, any zero address passed there will skip proper validation.

Impact: Medium

  • Non-standard behavior / silent misuse: Calls involving zero addresses won’t revert with the expected ERC20InvalidApprover / ERC20InvalidSpender errors, making misconfigurations harder to detect and leading to inconsistent error semantics.

  • Impact 2

Proof of Concept

  • Create poc9.t.sol under test directory and copy the code below.

  • Run forge test --mp poc9 -vvvv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {Token} from "./Token.sol";
import {IERC20Errors} from "../src/helpers/IERC20Errors.sol";
​
// @notice PoC: _spendAllowance lacks zero-address checks for owner/spender.
// In OpenZeppelin, zero-address validations are enforced (via _approve or internal checks).
// In this codebase, _spendAllowance:
// - computes allowance slot,
// - reverts ONLY on insufficient allowance (ERC20InsufficientAllowance),
// - never checks owner==0 or spender==0 to emit ERC20InvalidApprover/Spender.
contract SpendAllowanceZeroAddrTest is Test {
Token internal token;
​
function setUp() public {
token = new Token();
}
​
// @dev CASE 1: owner == address(0)
// transferFrom(0x0, to, 1) should revert with ERC20InvalidApprover(0x0),
// but actual behavior is a revert with ERC20InsufficientAllowance(spender, 0, 1).
function test_spendAllowance_ownerZero_revertsWithWrongError() public {
address spender = address(0xBEEF);
address to = address(0xCAFE);
​
// Low-level call to capture revert data
vm.prank(spender);
(bool ok, bytes memory ret) = address(token).call(
abi.encodeWithSelector(token.transferFrom.selector, address(0), to, 1)
);
​
assertFalse(ok, "call should revert");
// Decode revert selector (first 4 bytes of revert data)
bytes4 sel = _selector(ret);
assertEq(
sel,
IERC20Errors.ERC20InsufficientAllowance.selector,
"BUG: expected InvalidApprover, got InsufficientAllowance"
);
// Sanity: InsufficientAllowance payload is 0x64 bytes (selector + 3 arguments)
assertEq(ret.length, 0x64, "InsufficientAllowance revert length should be 0x64");
​
// Ensure it's NOT the expected InvalidApprover
assertTrue(
sel != IERC20Errors.ERC20InvalidApprover.selector,
"BUG: should NOT be InvalidApprover because zero-address checks are missing"
);
}
​
// @dev CASE 2: spender == address(0)
// By pranking from address(0), transferFrom(owner, to, 1) should revert with ERC20InvalidSpender(0x0),
// but actual behavior is a revert with ERC20InsufficientAllowance(0x0, 0, 1).
function test_spendAllowance_spenderZero_revertsWithWrongError() public {
address owner = address(0xA11CE);
address to = address(0xF00D);
​
// Mint some tokens to the owner (not strictly necessary; _spendAllowance fails first)
token.mint(owner, 10);
​
// Call as the zero address (Foundry allows pranking from address(0) for PoC)
vm.prank(address(0));
(bool ok, bytes memory ret) = address(token).call(
abi.encodeWithSelector(token.transferFrom.selector, owner, to, 1)
);
​
assertFalse(ok, "call should revert");
bytes4 sel = _selector(ret);
assertEq(
sel,
IERC20Errors.ERC20InsufficientAllowance.selector,
"BUG: expected InvalidSpender, got InsufficientAllowance"
);
assertEq(ret.length, 0x64, "InsufficientAllowance revert length should be 0x64");
​
assertTrue(
sel != IERC20Errors.ERC20InvalidSpender.selector,
"BUG: should NOT be InvalidSpender because zero-address checks are missing"
);
}
​
// --- helpers ---
​
// @dev Extracts the first 4 bytes (error selector) from ABI-encoded revert data.
function _selector(bytes memory data) internal pure returns (bytes4 sel) {
if (data.length < 4) return bytes4(0);
assembly {
sel := mload(add(data, 32)) // takes the first 4 bytes of the payload
}
}
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
​
Ran 2 tests for test/poc9.t.sol:SpendAllowanceZeroAddrTest
[PASS] test_spendAllowance_ownerZero_revertsWithWrongError() (gas: 16775)
Traces:
[16775] SpendAllowanceZeroAddrTest::test_spendAllowance_ownerZero_revertsWithWrongError()
├─ [0] VM::prank(0x000000000000000000000000000000000000bEEF)
│ └─ ← [Return]
├─ [3272] Token::transferFrom(0x0000000000000000000000000000000000000000, 0x000000000000000000000000000000000000cafE, 1)
│ └─ ← [Revert] ERC20InsufficientAllowance(0x000000000000000000000000000000000000bEEF, 0, 1)
├─ [0] VM::assertFalse(false, "call should revert") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(0xfb8f41b200000000000000000000000000000000000000000000000000000000, 0xfb8f41b200000000000000000000000000000000000000000000000000000000, "BUG: expected InvalidApprover, got InsufficientAllowance") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(100, 100, "InsufficientAllowance revert length should be 0x64") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertTrue(true, "BUG: should NOT be InvalidApprover because zero-address checks are missing") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
[PASS] test_spendAllowance_spenderZero_revertsWithWrongError() (gas: 62719)
Traces:
[62719] SpendAllowanceZeroAddrTest::test_spendAllowance_spenderZero_revertsWithWrongError()
├─ [45004] Token::mint(0x00000000000000000000000000000000000A11cE, 10)
│ └─ ← [Stop]
├─ [0] VM::prank(0x0000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [3272] Token::transferFrom(0x00000000000000000000000000000000000A11cE, 0x000000000000000000000000000000000000F00D, 1)
│ └─ ← [Revert] ERC20InsufficientAllowance(0x0000000000000000000000000000000000000000, 0, 1)
├─ [0] VM::assertFalse(false, "call should revert") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(0xfb8f41b200000000000000000000000000000000000000000000000000000000, 0xfb8f41b200000000000000000000000000000000000000000000000000000000, "BUG: expected InvalidSpender, got InsufficientAllowance") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(100, 100, "InsufficientAllowance revert length should be 0x64") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertTrue(true, "BUG: should NOT be InvalidSpender because zero-address checks are missing") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 8.44ms (4.13ms CPU time)
​
Ran 1 test suite in 126.94ms (8.44ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

Recommended Mitigation

  • Add explicit zero-address checks to _spendAllowance, with canonical custom errors. Keep the rest of the logic unchanged.

​
function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
- assembly ("memory-safe") {
+ assembly ("memory-safe") {
+ // Zero-address validations (match existing custom errors semantics)
+ if iszero(owner) {
+ // ERC20InvalidApprover(address approver)
+ mstore(0x00, shl(224, 0xe602df05))
+ mstore(add(0x00, 4), owner)
+ revert(0x00, 0x24)
+ }
+ if iszero(spender) {
+ // ERC20InvalidSpender(address spender)
+ mstore(0x00, shl(224, 0x94280d62))
+ mstore(add(0x00, 4), spender)
+ revert(0x00, 0x24)
+ }
+
let ptr := mload(0x40)
let baseSlot := _allowances.slot
mstore(ptr, owner)
mstore(add(ptr, 0x20), baseSlot)
let initialHash := keccak256(ptr, 0x40)
mstore(ptr, spender)
mstore(add(ptr, 0x20), initialHash)
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))
}
}

Support

FAQs

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

Give us feedback!