Arbitrarily set rubiconMarketAddress opens rug vector for admin or attacker with compromised keys

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L245-L247
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L286-L291

Vulnerability details

Impact

At any point, the owner of BathHouse.sol is able to change the address of the market contract by calling BathHouse.setBathTokenMarket() which calls BathToken.setMarket(). The new market address can be given max approval of all underlying tokens in the BathToken contract through a call to BathToken.approveMarket(). This creates a clear rug vector for the admin of the protocol, or any malicious user who has obtained compromised admin keys.

Proof of Concept

• Users deposit their underlying token into BathToken.sol
• Admin changes the rubiconMarketAddress
• Admin sets max approval for rubiconMarketAddress over the underlying tokens in the BathToken.sol contract.
• Admin can do whatever they like with all the underlying tokens.

Tools Used

Manual review.

Recommended Mitigation Steps

Apply a timelock for large scale changes similar to this one. Allow the users 24 hours to be able to withdraw their underlying tokens before the market address change is put into effect.

[bghughes]

Duplicate of #344

[HickupHH3]

Duplicate of #249