code423n4 commented 2 years ago

Handle

Jujic

Vulnerability details

There is no upper limit on poolColl.tokens[], it increments each time when a new collateral is added. Eventually, as the count of collateral increases, gas cost of smart contract calls will raise and that there is no implemented function to reduce the array size.

Impact

For every call getVC() function which computed contain the VC value of a given collateralAddress is listed in poolColl.tokens[] array, the gas consumption can be more expensive each time that a new collateral address is appended to the array, until reaching an "Out of Gas" error or a "Block Gas Limit" in the worst scenario.

Proof of Concept

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/ActivePool.sol#L268

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/DefaultPool.sol#L184

Tools Used

Remix

Recommended Mitigation Steps

Array's length should be checked.

kingyetifinance commented 2 years ago

@LilYeti: This is a known problem, and we are yet to test the upper limits of the contracts as is. Not sure how more theoretical issues like these are scored, but I would agree with that it is a medium to high risk based on how likely it is to happen * the potential effects. The worst possible outcome is that funds are locked in the protocol because it costs too much gas to do a withdrawal. We are still doing analysis on this, judges do what you want with this information.