seeu
high
ERC165Checker may revert instead of returning false in the contract L2StandardBridge.sol.
In optimism/packages/contracts/package.json#L76 it was found the outdated version 4.3.2 of @openzeppelin/contracts then imported by multiple contracts, particularly from L2StandardBridge.sol. This raises an issue when checking if a contract implements an interface and support of ERC165 since it may revert instead of returning false.
ERC165Checker.supportsInterface is intended to always return a boolean and not to revert. However, if a target contract fails to implement EIP-165 as intended, especially if it produces a value other than 0 or 1, ERC165Checker.supportsInterface might revert due to the fact that Solidity 0.8's abi.decode reverts if the bytes raw data overflow the target type.
The problem in the contract is that L2StandardBridge.sol uses ERC165Checker to check for support for an interface but then doesn't handle the lack of it by reverting.
The outdated version of @openzeppelin/contracts is the following: optimism/packages/contracts/package.json#L76
"@openzeppelin/contracts": "4.3.2"The contract L2StandardBridge.sol imports the outdated @openzeppelin/contracts in:
optimism/packages/contracts/contracts/L2/messaging/L2StandardBridge.sol#L10:
import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";It is also worth noting that the pragma solidity version of L2StandardBridge.sol is 0.8.9 since this issue is specific for the Solidity versions 0.8:
pragma solidity ^0.8.9;The code affected by this problematic is in the function finalizeDeposit:
function finalizeDeposit(
address _l1Token,
address _l2Token,
address _from,
address _to,
uint256 _amount,
bytes calldata _data
) external virtual onlyFromCrossDomainAccount(l1TokenBridge) {
// Check the target token is compliant and
// verify the deposited token on L1 matches the L2 deposited token representation here
if (
// slither-disable-next-line reentrancy-events
ERC165Checker.supportsInterface(_l2Token, 0x1d1d8b63) &&
_l1Token == IL2StandardERC20(_l2Token).l1Token()
) {
// When a deposit is finalized, we credit the account on L2 with the same amount of
// tokens.
// slither-disable-next-line reentrancy-events
IL2StandardERC20(_l2Token).mint(_to, _amount);
// slither-disable-next-line reentrancy-events
emit DepositFinalized(_l1Token, _l2Token, _from, _to, _amount, _data);
} else {
// Either the L2 token which is being deposited-into disagrees about the correct address
// of its L1 token, or does not support the correct interface.
// This should only happen if there is a malicious L2 token, or if a user somehow
// specified the wrong L2 token address to deposit into.
// In either case, we stop the process here and construct a withdrawal
// message so that users can get their funds out in some cases.
// There is no way to prevent malicious token contracts altogether, but this does limit
// user error and mitigate some forms of malicious contract behavior.
bytes memory message = abi.encodeWithSelector(
IL1ERC20Bridge.finalizeERC20Withdrawal.selector,
_l1Token,
_l2Token,
_to, // switched the _to and _from here to bounce back the deposit to the sender
_from,
_amount,
_data
);
// Send message up to L1 bridge
// slither-disable-next-line reentrancy-events
sendCrossDomainMessage(l1TokenBridge, 0, message);
// slither-disable-next-line reentrancy-events
emit DepositFailed(_l1Token, _l2Token, _from, _to, _amount, _data);
}
}The problematic arises when ERC165Checker.supportsInterface is called to check if a contract implements an interface and support of ERC165:
optimism/packages/contracts/contracts/L2/messaging/L2StandardBridge.sol#L153
ERC165Checker.supportsInterface(_l2Token, 0x1d1d8b63) &&Manual Review
It is highly suggested to update @openzeppelin/contracts to the most recent version, 4.8.1, or at least to the version 4.7.1 since the issue highlighted is patched from this version.
References:
Consider also that @openzeppelin/contracts's version contains other high severity vulnerabilities. See more here: