Era
ZKsync
FoundryLayer 2
500,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Use of standard ".approve" interface will cause bridging to fail for certain tokens

merklemore

Summary

L1ERC20Bridge.sol uses the standard ERC20 "approve" interface which returns a bool to handle token approvals in _approveFundsToAssetRouter but this will not work with certain tokens like USDT which do not return a bool on approval.

Vulnerability Details

The deposit function allows bridging to L2 and uses the _approveFundsToAssetRouter function to handle approvals to the router.

function deposit(
address _l2Receiver,
address _l1Token,
uint256 _amount,
uint256 _l2TxGasLimit,
uint256 _l2TxGasPerPubdataByte,
address _refundRecipient
) public payable nonReentrant returns (bytes32 l2TxHash) {
if (_amount == 0) {
// empty deposit amount
revert EmptyDeposit();
}
if (_l1Token == ETH_TOKEN_ADDRESS) {
revert ETHDepositNotSupported();
}
> uint256 amount = _approveFundsToAssetRouter(msg.sender, IERC20(_l1Token), _amount);
if (amount != _amount) {
// The token has non-standard transfer logic
revert TokensWithFeesNotSupported();
}
// ...

The _approveFundsToAssetRouter uses standard ".approve" method to approve the router to spend the amount to be bridged, expecting a bool in return.

function _approveFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) {
uint256 balanceBefore = _token.balanceOf(address(this));
_token.safeTransferFrom(_from, address(this), _amount);
> bool success = _token.approve(address(L1_ASSET_ROUTER), _amount);
if (!success) {
revert ApprovalFailed();
}
uint256 balanceAfter = _token.balanceOf(address(this));
​
return balanceAfter - balanceBefore;
}

But this will not work for tokens like USDT, BNB, OMG etc which are very popular in DeFi. Their implementation looks like this:

function approve(address usr, uint wad) external {
allowance[msg.sender][usr] = wad;
emit Approval(msg.sender, usr, wad);
}

And as a result, due to the interface mismatch will always revert.

Impact

Bridging such tokens like USDT will be impossible.

Tools Used

Manual Review.

Recommendations

Use the safeApprove or forceApprove functions instead.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`L1ERC20Bridge` Uses Unsafe Approvals - USDT won't work

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.