DittoETH

Summary

YieldFacet.sol::distributeYield() uses an unbounded loop. Unbounded loops may be exploited by malicious actors in Denial of Service attacks; particularly given that the functions has an External visibility modifier, making it accessible to anyone.

Vulnerability Details

In a DoS attack in this context, a malicious actor could intentionally call the combineShorts function with a very large ids array or the distributeYield() function with a very large assets array. This can cause the function to run for a long time, potentially consuming all the gas provided for the transaction. This could make the function impractical to use if the gas costs become prohibitively high.

Given that DOS on this function could prevent any yield from being distributed it is a fundamental break of the protocol, locking user's funds in.

Impact

If the attacker sends many of these large array transactions in quick succession, it could lead to network congestion. Other users trying to interact with the contract might find their transactions delayed or even priced out due to rising gas prices.
Regular users or even the contract's developers might find it too expensive in terms of gas to call the distributeYield() function, especially if they anticipate the attacker's transactions causing their own to fail which could deter them from using the function altogether.

Tools Used

Manual Review
Foundry

Recommendations

To mitigate this, a hard limit should be set on the number of iterations a loop can perform or limit the size of user-provided arrays.

distributeYield()

Creating a new uint parameter in the AppStorage struct in AppStorage.sol which is an updateable store of how many assets are in the protocol.
This can be referenced in a check against the array length passed into distributeYield().