Assessment reports>Y2K Finance>Medium findings>Malicious users can profit due to temporary exchange rate fluctuations
Category: Business Logic

Malicious users can profit due to temporary exchange rate fluctuations

Medium Severity
Medium Impact
Low Likelihood

Description

When a position is closed by calling closePosition, the deposit queue is cleared by pulling funds from the queue contract using the function _pullQueuedDeposits. The function _pullQueuedDeposits is only called when the length of queueDeposits is less than maxQueuePull.

If the length of this queueDeposits array is greater than maxQueuePull, the queue is first reduced using the function clearQueuedDeposits. The relevant part of the code is shown below:

function closePosition() external onlyOwner {
    if (!fundsDeployed) revert FundsNotDeployed();
    
    //...
    fundsDeployed = false;
    //...
    uint256 queueLength = queueDeposits.length;
    if (queueLength > 0 && queueLength < maxQueuePull)
        _pullQueuedDeposits(queueLength);
}

There may be a scenario where either the owner forgets to call the clearQueuedDeposits function before closePosition or a malicious user front-runs the owner's closePosition call to increase the length of the queue such that _pullQueuedDeposits is not called. In both these cases, the queue will not be cleared, but fundsDeployed would be set to false.

If the owner later tries to clear the queue by calling the function clearQueuedDeposits multiple times, the exchange rate would temporarily fluctuate. This is due to a bug in the function clearQueuedDeposits.

While clearing part of the queue, the function pulls all the funds from the QueueContract. At this time, the totalSupply is only increased by a small amount, but totalAssets is increased by a large amount. The exchange rate would again reach back to the expected amount when the entire queue is cleared, but between the calls to clearQueuedDeposits, the exchange rate is incorrect. A malicious user can profit by calling withdraw between these calls as they would receive more assets than they should.

Impact

A malicious user can sell shares at higher price than expected.

Recommendations

In the function clearQueuedDeposits, only the assets that are removed from the queue should be pulled from the QueueContract.

Remediation

The issue was fixed in commit 86e24fe.

Zellic © 2024Back to top ↑