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

It will be impossible to bridge USDT, and all other tokens that do not return bool on approval

Centaur

Summary

The deposit function in L1ERC20Bridge.sol expects a boolean return value from the approve function of ERC-20 tokens. However, many widely-used tokens (e.g., USDT) do not comply with this expectation, resulting in a revert when attempting to bridge such tokens. This issue makes the bridge contract incompatible with a significant portion of ERC-20 tokens, disrupting user operations.

Vulnerability Details

In the L1ERC20Bridge::deposit function, an internal call is made to _approveFundsToAssetRouter, where the token approval is handled as follows:

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);
//@audit will revert on usdt
if (!success) {
revert ApprovalFailed();
}
uint256 balanceAfter = _token.balanceOf(address(this));
return balanceAfter - balanceBefore;
}

The issue arises because not all ERC-20 tokens strictly follow the standard specification. Here is a snippet of USDT approve function deployed on L1.

function approve(address _spender, uint _value) public onlyPayloadSize(2 * 32) {
​
// To change the approve amount you first have to reduce the addresses`
// allowance to zero by calling `approve(_spender, 0)` if it is not
// already 0 to mitigate the race condition described here:
// https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
require(!((_value != 0) && (allowed[msg.sender][_spender] != 0)));
​
allowed[msg.sender][_spender] = _value;
Approval(msg.sender, _spender, _value);
}

The above function has no return statement, and will casuse a revert if the void returned from the call is assigned to a value just like it is done here;

bool success = _token.approve(address(L1_ASSET_ROUTER), _amount);

Impact

  • Users cannot bridge tokens like USDT and BNB

  • The protocol may lose transaction fees and volume due to incompatibility with popular tokens

Tools Used

Manual Review

Recommendations

Handle the approval success through a low level call, or use the openzeppelin safeApprove

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 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.