code423n4 commented 2 years ago

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 commented 2 years ago

Duplicate of #344

HickupHH3 commented 2 years ago

Duplicate of #249

code-423n4 / 2022-05-rubicon-findings