Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

ShadowForce - User can avoid paying deposit fee #318

Closed
sherlock-admin opened this issue Mar 27, 2023 · 5 comments
Closed

ShadowForce - User can avoid paying deposit fee #318

sherlock-admin opened this issue Mar 27, 2023 · 5 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

ShadowForce

medium

User can avoid paying deposit fee

Summary

user can bypass deposit fee if id is set to 0

Vulnerability Detail

In the _depostit function the logic charges a fee only if a the id is set to anything else but 0. A user can set the epoch id to 0 in the queue and completely avoid paying such deposit fee when he goes to mint. The protocol does not collect a deposit fee from user when in fact it should have collected said fee. We can view the logic down below

 function _deposit(
        uint256 _id,
        uint256 _assets,
        address _receiver
    ) internal {
        // mint logic, either in queue or direct deposit
        if (_id != 0) {
            uint256 assetsToDeposit = _assets;

            if (depositFee > 0) {
                (uint256 maxX, , uint256 minX) = getEpochConfig(_id);
                // deposit fee is calcualted linearly between time of epoch creation and epoch starting (deposit window)
                // this is because late depositors have an informational advantage
                uint256 fee = _calculateFeePercent(int256(minX), int256(maxX));
                // min minRequiredDeposit modifier ensures that _assets has high enough value to not devide by 0
                // 0.5% = multiply by 10000 then divide by 50
                uint256 feeAmount = _assets.mulDivDown(fee, 10000);
                assetsToDeposit = _assets - feeAmount;
                _asset().safeTransfer(treasury, feeAmount);
            }

            _mintShares(_receiver, _id, assetsToDeposit);

            emit Deposit(msg.sender, _receiver, _id, _assets);
        } else {
            depositQueue.push(
                QueueItem({assets: _assets, receiver: _receiver, epochId: _id})
            );

            emit DepositInQueue(msg.sender, _receiver, _id, _assets);
        }
    }

This can be further seen by looking at the snippet below. When shares are minted through deposit queue, no fees are charged

        // queue is executed from the tail to the head
        // get last index of queue
        uint256 i = length - 1;
        while ((length - _operations) <= i) {
            // this loop impelements FILO (first in last out) stack to reduce gas cost and improve code readability
            // changing it to FIFO (first in first out) would require more code changes and would be more expensive
            _mintShares(
                queue[i].receiver,
                _epochId,
                queue[i].assets - relayerFee
            );
            emit Deposit(
                msg.sender,
                queue[i].receiver,
                _epochId,
                queue[i].assets - relayerFee
            );
            depositQueue.pop();
            if (i == 0) break;
            unchecked {
                i--;
            }
        }

As we can see if user is depositing via que and not direct deposit, he can avoid paying fee by setting the epoch id to 0

Impact

The protocol does not collect a deposit fee from user when in fact it should have collected said fee. This is a direct loss of funds for the protocol.

Code Snippet

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/Carousel/Carousel.sol#L465-L501

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/Carousel/Carousel.sol#L328-L350

Tool used

Manual Review

Recommendation

we recommend the protocol adds checks to ensure a user pays fees when he should.

Duplicate of #75

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@dmitriia
Copy link
Collaborator

dmitriia commented Apr 9, 2023

Duplicate #75

@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue labels Apr 11, 2023
@Jiaren-tang
Copy link

Jiaren-tang commented Apr 11, 2023

Escalate for 10 USDC. Duplicate of #75 is a high finding and this issue should be rewarded.

@sherlock-admin
Copy link
Contributor Author

sherlock-admin commented Apr 11, 2023

Escalate for 10 USDC. Duplicate of #75 is a high finding and this issue should be rewarded.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Apr 11, 2023
@hrishibhat
Copy link

Escalation accepted

Valid duplicate of #75

@sherlock-admin
Copy link
Contributor Author

Escalation accepted

Valid duplicate of #75

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Apr 21, 2023
@hrishibhat hrishibhat removed the Non-Reward This issue will not receive a payout label Apr 28, 2023
@sherlock-admin sherlock-admin added High A valid High severity issue Reward A payout will be made for this issue labels Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

4 participants