Centrifuge - jaraxxus's results

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/LiquidityPool.sol#L258-L268

Vulnerability details

Proof of Concept

In LiquidityPool.sol, there is a collectDeposit and collectRedeem function with no access control. Anyone can call the function for another party to collect their funds.

    //@audit no access control
    function collectDeposit(address receiver) public {
        investmentManager.collectDeposit(receiver);
        emit DepositCollected(receiver);
    }

    /// @notice Trigger collecting the deposited tokens.
    function collectRedeem(address receiver) public {
        investmentManager.collectRedeem(receiver);
        emit RedeemCollected(receiver);
    }

This is a two-part issue, one being no access control over these functions and the second being a potential griefing of the protocol by sending dust amounts of deposits and redemption requests to the gateway to strain the process.

Impact

Anyone can call collectDeposit or collectRedeem for anyone, which should not be the case. A potential impact of having no access control is that anyone can spam the gateway by sending many requests with dust amounts, possibly causing a huge bottleneck of deposits for the router to clear.

Tools Used

Manual Review

Recommended Mitigation Steps

Add the withApproval() modifier on those functions too.

    function collectDeposit(address receiver) public withApproval(owner) {
        investmentManager.collectDeposit(receiver);
        emit DepositCollected(receiver);
    }


    /// @notice Trigger collecting the deposited tokens.
    function collectRedeem(address receiver) public withApproval(owner) {
        investmentManager.collectRedeem(receiver);
        emit RedeemCollected(receiver);
    }

This helps filter rogue individuals from straining the gateway. However, it doesn't really help if the user himself is malicious and intentionally calls collectDeposit with dust amounts everytime.

The best way is probably to have a minimum request sum so that users will not be able to grief the protocol by calling dust deposits everytime

Assessed type

Invalid Validation

Summary

• There is a liquidity pool that represents a real world asset (eg real estate, car, paintings)
• In the liquidity pool, there is a main currency accepted and a main tranche token accepted
• The pool can have more than one tranche (junior / senior)
• A tranche determines the risk/reward ratio. A junior tranche has higher risk but higher reward. A senior tranche has lower risk and lower rewards

For a user to participate

• Must be a member, checked in checkTransferRestriction()
• The user calls requestDeposit and deposits an arbitrary amount, eg 1000 USDC
• The protocol will receive the request. The user's money will be deposited into the escrow contract first
• The process is asynchronous, meaning that the protocol does not need to process the request immediately. Instead, the protocol waits for a period of time (an epoch) to process the request
• In the meantime (during the epoch period), the user can either decrease their sum deposited or cancel their deposit request
• Once the deposit is processed, the protocol will convert the deposit amount into tranche tokens and deposit the tokens into the escrow, depending on the exchange rate. Eg, the user's 1000 USDC is worth 750 tranche tokens, meaning 1 tranche token is worth 1.25 USD at that point in time.
• The user can then call deposit the withdraw the tranche tokens from the escrow. Keep in mind that the 1000 USDC has already been converted to tranche tokens, so the user can only withdraw tranche tokens
• To redeem the tranche tokens back to the native currency of the pool, eg USDC, the whole process is repeated (user calls requestRedeem)

Codebase quality analysis

• Codebase is written well, and most edge cases is checked even with extremely high composability
• Decimals are calculated properly in the investment manager

Centralization risks

• Huge centralization risk when it comes to Auth. Anyone with a ward status can deny everyone else of the ward status, including himself. Since the whole protocol revolves around Auth, if there is no wards in the protocol, every process will not function anymore.
• The router and the centrifuge chain must encode and decode messages properly because the whole process relies on converting native currency to tranche tokens and back. The centrifuge chain must be fair in allocating the price of the tranche tokens

Mechanism Review

• The process of receiving tranche tokens by depositing native currency can be extremely long, especially if the epoch period is long. Users have to understand that they may have to wait for a long time, and they may not get the best conversion rate.

Time spent:

10 hours