zkSync Era - max10afternoon's results

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/bridge/L1ERC20Bridge.sol#L278

Vulnerability details

Impact

The L1 erc20 tokens bridge has a claimFailedDeposit function that allows to withdraw the tokens in cases where the deposit transaction on L2 were to fail. Also a bridge might have a deposit limit, which can be added or modified at any time. Adding a deposit limit for a token that doesn't have any, will cause the claimFailedDeposit function to revert with an underflow for every deposit that has been processed before the update.

Proof of Concept

The deposit function of the L1ERC20Bridge contract internally calls the _verifyDepositLimit function, which verifies and update the deposit limit for the token being transferred by the user, if existent:

        IAllowList.Deposit memory limitData = IAllowList(allowList).getTokenDepositLimitData(_l1Token);
        if (!limitData.depositLimitation) return; // no deposit limitation is placed for this token

        if (_claiming) {
            totalDepositedAmountPerUser[_l1Token][_depositor] -= _amount;
        } else {
            require(totalDepositedAmountPerUser[_l1Token][_depositor] + _amount <= limitData.depositCap, "d1");
            totalDepositedAmountPerUser[_l1Token][_depositor] += _amount;
        }

As can be seen the totalDepositedAmountPerUser[_l1Token][_depositor] variable, that keeps track of how many token each user has deposited, only gets updated if the deposit limit exist for the token.

The L1ERC20Bridge contract also has a claimFailedDeposit function that allows to withdraw tokens on L1 in case where the L2 transaction fails. This function will also internally call _verifyDepositLimit and update the deposit limit, by decreasing the amount deposited by the user.

If during the deposit the token doesn't have deposit limit the totalDepositedAmountPerUser[_l1Token][_depositor] variable for the user won't be updated; therefor if the owner adds a deposit limit afterwards the _verifyDepositLimit call of the claimFailedDeposit function will revert with an underflow, since it will try to subtract the amount being withdraw from the default value of a uint256 which is 0. Preventing the user from getting the tokens back.

-The owner can update deposit limits by calling the setDepositLimit function of the AllowList contract, which is set in the constructor of the ERC20 bridge

Assessed type

Other