code-423n4 / 2024-05-olas-validation

0 stars 0 forks source link

The contract `OptimismTargetDispenserL2` uses incorrect `l1Processor` #166

Open c4-bot-1 opened 3 months ago

c4-bot-1 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/OptimismTargetDispenserL2.sol#L101

Vulnerability details

Impact

OptimismTargetDispenserL2 contract is used by te protocol to receive the messages and tokens from L1 deposit processors contracts and then complete a verification process of staking targets and send the funds to them. However, the contract uses incorrect l1Processor address when receiving the message.

Proof of Concept

According to the Optimism docs:

https://docs.optimism.io/chain/differences

When transactions are sent from L1 to L2 by an Externally Owned Account, the address of the sender of the transaction on L2 will be set to the address of the sender of the transaction on L1. However, the address of the sender of a transaction on L2 will be different if the transaction was triggered by a smart contract on L1.

So the transactions of sending the messages from L1 to L2 are implemented by the smart contract, therefore, the address should be this:

L1_contract_address + 0x1111000000000000000000000000000000001111

However, in the current OptimismTargetDispenserL2, it uses the following sender:

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/OptimismTargetDispenserL2.sol#L96-102

 function receiveMessage(bytes memory data) external payable {
        // Check for the target dispenser address
        address l1Processor = IBridge(l2MessageRelayer).xDomainMessageSender();

        // Process the data
        _receiveMessage(msg.sender, l1Processor, data);
    }

The contract deviates from Optimism docs and uses incorrect sender address (not "aliased" version of L1 sender address).

Tools Used

Manual review.

Recommended Mitigation Steps

Add offset the same way as in ArbitrumTargetDispenserL2 smart contract:

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/ArbitrumTargetDispenserL2.sol#L46-49

 uint160 offset = uint160(0x1111000000000000000000000000000000001111);
        unchecked {
            l1AliasedDepositProcessor = address(uint160(_l1DepositProcessor) + offset);
        }

Assessed type

Other

rodiontr commented 3 months ago

@0xA5DF I think this issue is valid as in the case of Arbitrum the protocol uses correct "aliased" version of L1 sender address. But in the case of Optimism, it's not aliased and it's deviation from Optimism spec