Approval of zero address to spend "zombie funds" possible
Report ID
#21968
Target
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:
- 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.
- Token Manipulation: If the ecrecover silent failure is present in a token contract, an attacker could exploit it to manipulate token balances or allowances.
- 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
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.