sherlock-audit / 2023-03-Y2K-judging

7 stars 1 forks source link

ak1 - Carousel.sol : `enlistInRollover` is not increasing the asset value when already queued receiver calls again `enlistInRollover` #484

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

high

Carousel.sol : enlistInRollover is not increasing the asset value when already queued receiver calls again enlistInRollover

Summary

Carousel.sol contract has the function enlistInRollover, to ennlist the particular epoch for a particular reciever. This function is not increasing the asset value if the already enlisted receiver calls. Instead it is overwriting the asset value.

Vulnerability Detail

Below is the portion of enlistRollover function

function enlistInRollover(
    uint256 _epochId,
    uint256 _assets,
    address _receiver
) public epochIdExists(_epochId) minRequiredDeposit(_assets) {
    // check if sender is approved by owner
    if (
        msg.sender != _receiver &&
        isApprovedForAll(_receiver, msg.sender) == false
    ) revert OwnerDidNotAuthorize(msg.sender, _receiver);
    // check if user has enough balance
    if (balanceOf(_receiver, _epochId) < _assets)
        revert InsufficientBalance();

    // check if user has already queued up a rollover
    if (ownerToRollOverQueueIndex[_receiver] != 0) { ------------------> already queued.
        // if so, update the queue
        uint256 index = getRolloverIndex(_receiver);
        rolloverQueue[index].assets = _assets; ------------------------> asset value is overwritten
        rolloverQueue[index].epochId = _epochId;
    } else {

Impact

Loss of asset.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L238-L258

Tool used

Manual Review

Recommendation

Increment the asset value instead of assigning.