DittoETH

Summary

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

Vulnerability Details

In a DoS attack in this context, a malicious actor could intentionally call the combineShorts function with a very large ids. 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.

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 combineShorts 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.

combineShorts()

Update the required max length of ids. This can be some arbitrary number based on likely amount of short records a user will want to combine - have put 10 as max below.