smartcontractkit / smartcontractkit/chainlink-evm

Clarification: should frozen ERC20 spenders be blocked from pre-existing infinite allowances?

Open
#470 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
82
Forks
27
Avg merge
1d 20h
Merged PRs (30d)
24

Description

Hi Chainlink team,

I'm reviewing the freeze semantics in the CCIP EVM Freezable BurnMint ERC20 contracts and noticed a difference between finite and infinite allowances after a spender is frozen.

Affected contracts:

  • BurnMintERC20PausableFreezableUUPS
  • BurnMintERC20PausableFreezableTransparent

Summary

The contracts block frozen accounts in:

  • _update(from, to, value), when the frozen account is from or to
  • _approve(owner, spender, value, emitEvent), when the frozen account is owner or spender

This means a frozen spender cannot receive a new approval, and finite allowance spends revert because OpenZeppelin's _spendAllowance decreases the allowance through _approve.

However, OpenZeppelin ERC20 v5 skips _approve when the current allowance is type(uint256).max.

As a result, a spender that was approved for infinite allowance before being frozen can still call transferFrom and move third-party funds, as long as neither from nor to is frozen.

Example flow

// 1. Alice approves Spender before Spender is frozen
token.approve(spender, type(uint256).max);

// 2. Spender is later frozen
token.freeze(spender);

// 3. Spender calls transferFrom
token.transferFrom(alice, bob, amount);

Current result:

  • _spendAllowance(alice, spender, amount) does not call _approve because allowance is max.
  • The frozen-spender check in _approve is skipped.
  • _update(alice, bob, amount) only checks alice and bob.
  • If neither alice nor bob is frozen, the transfer succeeds.

For finite allowances, the same flow reverts because _spendAllowance calls _approve, which detects the frozen spender.

Clarification requested

Is this intended behavior?

More specifically, is the intended freeze invariant only:

A frozen account cannot be the from or to address in transfers, mints, or burns.

Or should the invariant also include:

A frozen account cannot act as msg.sender / spender and spend third-party allowances through transferFrom.

The ambiguity comes from _approve rejecting frozen spenders, which suggests frozen spenders are intended to be blocked from allowance-based flows. But infinite allowances bypass that check.

Related burnFrom behavior

The same pattern may apply to burnFrom if the frozen caller still has the required burn role and has a pre-existing infinite allowance, because burnFrom also relies on _spendAllowance.

Possible fix if unintended

If frozen spenders should not be able to spend allowances, the contract could override _spendAllowance and check the spender before OpenZeppelin's infinite-allowance shortcut:

function _spendAllowance(address owner, address spender, uint256 value) internal virtual override {
    if (isFrozen(spender)) {
        revert AccountFrozen(spender);
    }

    super._spendAllowance(owner, spender, value);
}

Exact error names may differ, but the key point is that the spender freeze check should happen before the max-allowance branch.

Can the Chainlink team confirm whether frozen accounts being able to spend pre-existing infinite allowances is intended behavior?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with BurnMintERC20PausableFreezableUUPS and BurnMintERC20PausableFreezableTransparent, then trace _approve, _spendAllowance, transferFrom, and burnFrom under OpenZeppelin ERC20 v5. Confirm the intended freeze invariant for pre-existing infinite allowances; done means the team has decided the behavior and the relevant regression coverage reflects that decision.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
blockchain, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.