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

kenzo - Depositor can totally bypass deposit fee using deposit queue #175

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label 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

kenzo

high

Depositor can totally bypass deposit fee using deposit queue

Summary

When the deposit is queue is being minted, the deposit fee is not deducted from the deposit assets.
Therefore the deposit fee is bypassable, losing funds for the protocol.

Vulnerability Detail

In the regular deposit function, if the user requested to deposit in a certain epoch (_id != 0), a deposit fee is being substracted from the amount that the user will receive.

        if (_id != 0) {
            uint256 assetsToDeposit = _assets;
            if (depositFee > 0) {
                (uint256 maxX, , uint256 minX) = getEpochConfig(_id);
                uint256 fee = _calculateFeePercent(int256(minX), int256(maxX));
                // 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);

But if the epoch id supplied is 0, then the request will just be added to the deposit queue.

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

Later, when minting via mintDepositInQueue, we can see that no deposit fee is being deducted from the amount.

        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--;
            }
        }

Therefore, by choosing to deposit for the next epoch using the deposit queue, instead of the regular deposit function, a user can totally bypass the deposit fee.

Also note that since the deposit queue is "first in last out", and that whoever calls mintDepositInQueue will get the relayer fee, the user can straight away call it, mint his queued deposit and not pay the relayer fee either.

Impact

A majority of protocol fees can be circumvented, leading to substantial loss of profit for the protocol.
Since this seems to fall under the category of "would result in a material loss of funds, and the cost of the attack is low", and we are not talking about an hypothetical scenario, I labeled this as high severity.

Code Snippet

See above.

Tool used

Manual Review

Recommendation

Change the queue mechanism so fees would be taken there as well.

Duplicate of #75

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 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 High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant