113.md

Uneven Leather Salamander

Medium

Challengers may suffer losses due to overcharged bonds

Summary

When a challenger initiate a challenge against a batch, there's a check that ensure that the bond being paid is the required challengeDeposit(). But there's a condition that allows challenger's bonds to be overcharged, while it's unnecessary. The main goal of such check is to make sure that the required amount is paid, but the code allows any amount of ETH being sent due to the condition leading to potential losses for challengers.

Vulnerability Detail

Within the challengeState function the challenger bond is checked as follows:

        require(msg.value >= IL1Staking(l1StakingContract).challengeDeposit(), "insufficient value");

While the exact bond amount (1 ETH) is being retrieved from the L1Staking contract, the check should ensure that the challenger is depositing only the required amount for the bond (which is a fixed amount). But the condition explicitly allows any value above the required amount. To resume the code allows the challenger being overcharged due to the explicit "greater than" the bond value, while == would avoid any excess amount being sent by challengers and ultimately being lost.

Impact

Challengers could endup losing more than the required bond.

Code Snippet

https://github.com/sherlock-audit/2024-08-morphl2/blob/98e0ec4c5bbd0b28f3d3a9e9159d1184bc45b38d/morph/contracts/contracts/l1/rollup/Rollup.sol#L375

Tool used

Manual Review

Recommendation

Ensure that the check uses == instead of >=, the new condition would only accept the required ETH (bond amount)

-         require(msg.value >= IL1Staking(l1StakingContract).challengeDeposit(), "insufficient value");
+        require(msg.value == IL1Staking(l1StakingContract).challengeDeposit(), "incorrect value");