changeRelayerFee SHOULD BE CONTROLLED BY A TIMELOCK, ELSE USER TRANSACTION COULD BE REVERTED
Summary
In the Carousel.sol contract the changeRelayerFee function is used to change the relayerFee value of the contract. relayerFee is a critical value in the contract since both deposit and depositETH functions are restricted by the minRequiredDeposit(_assets) modifier. This modifier reverts if the depositing assets amount is less than the relayerFee. Hence the asset amount deposited using the deposit or the depositETH functions should be more than the relayerFee.
Vulnerability Detail
So the current fund deposit transactions could be reverted under following conditions
The relayerFee is changed by the CarouselFactory.sol contract by calling the changeRelayerFee function.
relayerFee is changed to a higher value than the existing relayerFee value.
Above transaction front runs the current transactions which are trying to deposit funds to the contract using the old relayerFee.
So when the deposit transactions are executed they are checked against the new relayerFee, even though the deposits were made during the time old relayerFee was existent.
Impact
So when the pending deposit transactions in the mempool are executed they are checked against the new relayerFee and could be reverted if the new relayerFee is more than the amount that the user is depositing. In this case not only the user loses his transaction gas fee but also will be confused as to why his transaction got reverted.
Code Snippet
function deposit(
uint256 _id,
uint256 _assets,
address _receiver
)
public
override(VaultV2)
epochIdExists(_id)
epochHasNotStarted(_id)
minRequiredDeposit(_assets)
nonReentrant
{
...
}
Apply a time lock functionality for the changeRelayerFee function, so the relayerFee change will only take place after a delay so user will have enough to time to prepare for the change.
Udsen
medium
changeRelayerFee
SHOULD BE CONTROLLED BY A TIMELOCK, ELSE USER TRANSACTION COULD BE REVERTEDSummary
In the
Carousel.sol
contract thechangeRelayerFee
function is used to change therelayerFee
value of the contract.relayerFee
is a critical value in the contract since bothdeposit
anddepositETH
functions are restricted by theminRequiredDeposit(_assets)
modifier. This modifier reverts if the depositing assets amount is less than therelayerFee
. Hence the asset amount deposited using thedeposit
or thedepositETH
functions should be more than therelayerFee
.Vulnerability Detail
So the current fund deposit transactions could be reverted under following conditions
relayerFee
is changed by theCarouselFactory.sol
contract by calling thechangeRelayerFee
function.relayerFee
is changed to a higher value than the existingrelayerFee
value.So when the deposit transactions are executed they are checked against the new
relayerFee
, even though the deposits were made during the time oldrelayerFee
was existent.Impact
So when the pending deposit transactions in the mempool are executed they are checked against the new
relayerFee
and could be reverted if the newrelayerFee
is more than the amount that the user is depositing. In this case not only the user loses his transaction gas fee but also will be confused as to why his transaction got reverted.Code Snippet
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L87
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L107
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L602-L604
Tool used
VSCode and Manual Review
Recommendation
Apply a time lock functionality for the
changeRelayerFee
function, so therelayerFee
change will only take place after a delay so user will have enough to time to prepare for the change.