Beedle - Oracle free perpetual lending

Summary

The sellProfits function in the Fees contract is vulnerable to a sandwich attack. In a normal scenario I would say that an attacker can monitor transactions on the blockchain, detect profitable trades, and front-run the intended swap with their own transaction to exploit the price difference. But this doesn’t totally apply in case of the sellProfits function, because the function is public and lacks an onlyOwner modifier, making it callable by anyone. Thus, Alice doesn’t need to monitor the transactions on the blockchain, she only needs to monitor the balances of the various tokens used within the contract, then, when a profitable scenario appears, she just executes her sandwich attack by calling the sellProfits on her own.

Vulnerability Details

The sellProfits function can be called publicly while lacking any access control mechanisms like an onlyOwner modifier. Malicious actors could make this function part of their trades to extract profit by sandwitching it when it's profitable enough.

Impact

Loss of funds due to lack of Access Control. Function can be called within a sandwich attack or simply called when there’s a momentarily dip in the market by malicious actors.

Tools Used

Manual review

Recommendations

Restrict access to the sellProfits function to authorized parties only by using onlyOwner modifier. Also this function is never called internally, thus it can be restricted to external.

If possible, avoid revealing the exact swap parameters in the transaction before it is confirmed. This can be achieved through commit-reveal schemes or using other privacy techniques.

Moreover Uniswap documentation (https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps) specifies the following:
"amountOutMinimum: we are setting to zero, but this is a significant risk in production. For a real deployment, this value should be calculated using our SDK or an onchain price oracle - this helps protect against getting an unusually bad price for a trade due to a front running sandwich or another type of price manipulation

sqrtPriceLimitX96: We set this to zero - which makes this parameter inactive. In production, this value can be used to set the limit for the price the swap will push the pool to, which can help protect against price impact or for setting up logic in a variety of price-relevant mechanisms."

Please don't hardcode these two parameters to 0 and follow Uniswap recommendations