msg.sender has to be un-aliased in L1ERC20Bridge.tranferTokenToSharedBridge()

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/bridge/L1ERC20Bridge.sol#L64

Vulnerability details

Impact

Users who deposit tokens in L1ERC20Bridge will not receive them on the shared Bridge because the check require(msg.sender == address(sharedBridge) will always revert.

Vulnerability details

L1ERC20Bridge.tranferTokenToSharedBridge() checks that msg.sender is sharedBridge:

require(msg.sender == address(sharedBridge), "Not shared bridge"); 

However, since the function is called directly as a L1ERC20Bridge - sharedBridge transaction initiated by sharedBridge, which is a contract, msg.sender will actually be the aliased address of sharedBridge. sharedBridge is the un-aliased address of the bridge. As such, this check will always revert, thus users who deposit tokens in L1ERC20Bridge will not receive them on the shared Bridge. L1ERC20Bridge.sol#L64

function tranferTokenToSharedBridge(address _token, uint256 _amount) external {
@>        require(msg.sender == address(sharedBridge), "Not shared bridge");
        uint256 amount = IERC20(_token).balanceOf(address(this));
        require(amount == _amount, "Incorrect amount");
        IERC20(_token).safeTransfer(address(sharedBridge), amount);
    }

The vulnerability is similar to spearbit-blast-report-1 where the check require(msg.sender == address(OTHER_BRIDGE),""); will always revert as msg.sender will be the aliased address of L1BlastBridge and OTHER_BRIDGE is the un-aliased address of L1BlastBridge.

Tools Used

Manual Review

Recommended Mitigation Steps

Un-alias msg.sender in the check as such:

-- require(msg.sender == address(sharedBridge), "Not shared bridge");
++ require(AddressAliasHelper.undoL1ToL2Alias(msg.sender) == address(sharedBridge), "Not shared bridge");

Assessed type

Error

[c4-sponsor]

saxenism (sponsor) disputed

[saxenism]

(De)Aliasing is not required for L1 to L1 contract interactions. The SharedBridge in question is the L1SharedBridge and the L1ERC20Bridge is also an L1 contract.

This is an L1 contract that interacts with another L1 contract 🙂

[alex-ppg]

The Warden details that the msg.sender of the L1ERC20Bridge::transferTokenToSharedBridge function needs to be un-aliased, however, as the Sponsor has described and the code highlights, the msg.sender is an L1 address compared to the L1 address of the IL1SharedBridge. As such, this exhibit is invalid given that validation behaves as expected.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid