Crazy Jetblack Pigeon
High
According to the ERC20 standard, the functions name, symbol, and decimals are optional, and no one should expect these functions to be present in a contract.
The _deposit function in the L1StandardERC20Gateway contract calls the name, symbol, and decimals methods to retrieve the name, symbol, and decimals of the ERC20 token. If a valid ERC20 token that does not implement these functions is passed, the call will fail, preventing the user from depositing the tokens.
function _deposit(
address _token,
address _to,
uint256 _amount,
bytes memory _data,
uint256 _gasLimit
) internal virtual override nonReentrant {
require(_amount > 0, "deposit zero amount");
// 1. Transfer token into this contract.
address _from;
(_from, _amount, _data) = _transferERC20In(_token, _amount, _data);
// 2. Generate message passed to L2StandardERC20Gateway.
address _l2Token = tokenMapping[_token];
bytes memory _l2Data;
if (_l2Token == address(0)) {
// @note we won't update `tokenMapping` here but update the `tokenMapping` on
// first successful withdraw. This will prevent user to set arbitrary token
// metadata by setting a very small `_gasLimit` on the first tx.
_l2Token = getL2ERC20Address(_token);
// passing symbol/name/decimal in order to deploy in L2.
string memory _symbol = IERC20MetadataUpgradeable(_token).symbol();
string memory _name = IERC20MetadataUpgradeable(_token).name();
uint8 _decimals = IERC20MetadataUpgradeable(_token).decimals();
_l2Data = abi.encode(true, abi.encode(_data, abi.encode(_symbol, _name, _decimals)));
} else {
_l2Data = abi.encode(false, _data);
}
bytes memory _message = abi.encodeCall(
IL2ERC20Gateway.finalizeDepositERC20,
(_token, _l2Token, _from, _to, _amount, _l2Data)
);
uint256 nonce = IL1CrossDomainMessenger(messenger).messageNonce();
// 3. Send message to L1CrossDomainMessenger.
IL1CrossDomainMessenger(messenger).sendMessage{value: msg.value}(counterpart, 0, _message, _gasLimit, _from);
emit DepositERC20(_token, _l2Token, _from, _to, _amount, _data, nonce);
}The impact is high because deposits will never be possible, as _l2Token == address(0) will always evaluate to true.
Manual Review
Below is the right implementation.
// decimals, symbol & token are not part of the core ERC20 token standard, so we need to support contracts that dont implement them
(,bytes memory queriedDecimals) = tokenAddress.staticcall(abi.encodeWithSignature("decimals()"));
(,bytes memory queriedSymbol) = tokenAddress.staticcall(abi.encodeWithSignature("symbol()"));
(,bytes memory queriedName) = tokenAddress.staticcall(abi.encodeWithSignature("name()"));
uint8 decimals = abi.decode(queriedDecimals, (uint8));
string memory symbolString = abi.decode(queriedSymbol, (string));
string memory nameString = abi.decode(queriedName, (string));