code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

User can provide collateral of any type instead of liquid staking tokens that protocol assumed to receive. #185

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/proxies/ODSafeManager.sol#L118-L133

Vulnerability details

Impact

If the collateral provided by the user, is a token with low value and liquidity, then it might affect protocol and the user itself in different ways including making the safe undercollateralized.

Proof of Concept

The openSafe function of ODSafeManager takes in two parameters _cType and _usr, it is wrongly assumed by the protocol that the collateral provided by the user is indeed the collateral he is supposed to provide. There is no whitelisting of collateral tokens but it is mentioned that the collateral should be liquid staking tokens and moreover, they will deploy with the following tokens ARB, WSTETH, CBETH, RETH, and MAGIC after that, new collaterals need to be voted in by the DAO, it can further be queried by calling in CollateralJoinFactory::collateralTypesList() also, in CollateralAuctionHouseFactory::collateralList()

The openSAFE function in the ODSafeManager does not directly handle the collateral. Instead, it creates a new SAFE (Single Collateral Debt Position) and assigns it to a user. The type of collateral that can be used is determined by the _cType parameter, which is a bytes32 value representing the collateral type. But nothing is stopping the user from providing collateral of low value and low liquidity which can lead to several potential issues.

  1. If collateral loses value rapidly, SAFE could become undercollateralized. This means the value of the debt in the SAFE is greater than the value of the collateral. In this case, SAFE could be liquidated.

  2. If the coin has low liquidity, it might be difficult to sell the collateral in the event of a liquidation, which could potentially cause issues in the system.

  3. It makes the whole purpose of the protocol absurd as it intends to provide debt against liquid staking tokens.

Tools Used

Manual Review

Recommended Mitigation Steps

Use whitelisting of collateral tokens that are allowed in the system as well as allowed for taking debt, and if the collateral is not from the list revert the transaction.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #90

c4-judge commented 1 year ago

MiloTruck changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

MiloTruck marked the issue as grade-b