📄

Report #21968

Report Date
July 6, 2023
Status
Closed
Payout

Approval of zero address to spend "zombie funds" possible

Report Info

Report ID

#21968

Target

https://etherscan.io/address/0x77700005bea4de0a78b956517f099260c2ca9a26

Report type

Smart Contract

Impacts

  • Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
  • Temporary freezing of funds for at least 1 hour
  • Permanent freezing of unclaimed yield

Has PoC?

Yes

Bug Description

The ecrecover precompile has a known issue that it fails silently. If there is no check that owner address is not equal to zero, it can return the zero address (address(0)) instead of an error when given a malformed message.

Beanstalk employs self permit in Root Token contract and TokenFacet.sol via the following functions:

    function permitToken(
        address owner,
        address spender,
        address token,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external payable nonReentrant {
        LibTokenPermit.permit(owner, spender, token, value, deadline, v, r, s);
        LibTokenApprove.approve(owner, spender, IERC20(token), value);
    }

and

function mintWithTokensPermit() and function mintWithTokenPermit()

These in turn interact with following libraries LibTokenPermit.permit()LibSiloPermit()

    function permit(
        address owner,
        address spender,
        address token,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) internal {
        require(block.timestamp <= deadline, "Token: permit expired deadline");
        bytes32 structHash = keccak256(abi.encode(TOKEN_PERMIT_TYPEHASH, owner, spender, token, value, _useNonce(owner), deadline));
        bytes32 hash = _hashTypedDataV4(structHash);
        address signer = ECDSA.recover(hash, v, r, s);
        require(signer == owner, "Token: permit invalid signature");
    }

All permits check that signer == owner, but does not check that the signer != address(0).

The warning is found in the Security Considerations of https://eips.ethereum.org/EIPS/eip-2612

Since the ecrecover precompile fails silently and just returns the zero address as signer when given malformed messages, it is important to ensure owner != address(0) to avoid permit from creating an approval to spend “zombie funds” belong to the zero address. - https://eips.ethereum.org/EIPS/eip-2612

The signer == owner check alone is not sufficient to ensure the validity of the signature in all cases. There are scenarios where the ecrecover function may fail to recover a valid Ethereum address, and in those cases, it returns the zero address (address(0)).

If you rely solely on the signer == owner check and ignore the signer != address(0) check, an attacker could craft a message or signature in a way that causes the ecrecover function to fail and return the zero address. In this case, the check signer == owner would pass, even though the signature is invalid.

Impact

In this context, if the owner address is mistakenly set to the zero address (address(0)), a potential vulnerability may arise. Although I have not developed specific POC's, wrong signature vulnerabilities in the wild have been known to lead to the following attacks:

  1. Theft of Funds: An attacker could craft a specially crafted message that triggers the silent failure of ecrecover, causing the zero address to be considered as the signer.
  2. Token Manipulation: If the ecrecover silent failure is present in a token contract, an attacker could exploit it to manipulate token balances or allowances.
  3. Arbitrary Actions: In contracts where certain actions or operations require a specific signature verification, the silent failure of ecrecover could bypass these checks. This could enable an attacker to execute arbitrary actions on behalf of the zero address.

Risk Breakdown

Difficulty to Exploit: Easy Weakness: CVSS2 Score:

Recommendation

Add the following: signer != address(0) && signer == owner to all permit functions.

Here is a Uniswap reference implementation that checks for both zero address and owner:

function permit(
        address owner,
        address spender,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external {
        require(deadline >= block.timestamp, "UniswapV2: EXPIRED");
        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                DOMAIN_SEPARATOR,
                keccak256(
                    abi.encode(PERMIT_TYPEHASH, owner, spender, value, nonces[owner]++, deadline)
                )
            )
        );
        address recoveredAddress = ecrecover(digest, v, r, s);
        require(
            recoveredAddress != address(0) && recoveredAddress == owner,
            "UniswapV2: INVALID_SIGNATURE"
        );
        _approve(owner, spender, value);
    }

References

https://eips.ethereum.org/EIPS/eip-2612

Proof of concept

POC not required, self-explanatory

BIC Response

This is not a valid bug report because the suggested check (if (signer == address(0))) is done within the ECDSA library itself. See line 154: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol#L154.

For these reasons, we are closing the submission and no reward will be issued.