DOS with unbounded loop

[code423n4]

Handle

Koustre

Vulnerability details

Impact

In UniswapHandler, in the function removeBuyer there is a for loop over an unbounded Buyers array, which if the buyers array gets too large can cause a denial of service and prevents the contract from being able to remove buyer roles from users/contracts. This would allow users/contracts to circumvent recovery mode and to continue to purchase and sell tokens using the contract.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

• Manual Study

Recommended Mitigation Steps

• remove unbounded for loop

[0xScotch]

This will not allow circumventing recovery mode unless a malicious buyer is explicitly added by an admin. The bug doesn't impact funds but does impact protocol availability which by label definitions should be medium not high.

[GalloDaSballo]

The finding mentions an unbound loop on removeBuyer, if the array reaches too big of a size you could go over the gas limit for a block and as such any function would revert, that is possible.

However, this is an external function, as such bricking it would have no impact on the rest of the contract.

Additionally, because the protocol uses Uniswap, even if the protocol was bricked, the uniswap pools would still work.

Lastly, in the incredibly unlikely scenario of reaching the gas limit, bricking this function simply means you won't be able to remove Buyers from the protocol. Which would only impact the buyMalt function, which is just a utility function to buy from the Uniswap Pool.

As such the impact of the finding is effectively nil.

I will downgrade to Low Severity because the warden has shown a way to brick a function, but the economic and availability impact is not meaningful in my opinion