Skip to content

First Depositor can be front-run - Improper Pool Initialization Bug #103

Open

Description

@code423n4

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/tokens/cCash/CTokenCash.sol#L357

Vulnerability details

Impact

Where: [CTokenCash.sol]

(https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/tokens/cCash/CTokenCash.sol#L357). In turn this means all the pTokens have this bug.

When: cToken.totalSupply == 0.

This attack has two implications

  • The "first" deposit can be front run and stolen Let's assume there is a first user trying to mint some cToken using their k*z underlying tokens

  • An attacker can see this transaction and carry out the above-described attack making sure that k<1. This leads to the first depositor getting zero cTokens for their k*z underlying tokens. All the tokens are redeemable by the attacker using their 1 wei of cToken.

  • Now, the attacker can treat the second deposit as the first one and repeat the same attack untill the second scenario mentioned below happens.

  • Implicit minimum Amount and funds lost due to rounding errors. If an attacker is successful in making 1 wei of pToken worth z underlying tokens and a user tries to mint pTokens using k* z underlying tokens then,

  • If k<1, then the user gets zero pTokens and all of their underlying tokens get proportionally divided between pToken holders This leads to an implicitminimum amount for a user at the attacker's discretion.

  • If k>1, then users still get some pTokens but they lose (k- floor(k)) * z) of underlying tokens which get proportionally divided between pToken holders due to rounding errors.

This means that for users to not lose value, they have to make sure that k is an integer. Attacker can listen to [MarketListed] from Unitroller to front-run.

Proof of Concept

When: cToken.totalSupply == 0.

  • When totalSupply is zero an attacker goes ahead and executes the following steps.
  • They use some underlying tokens (i.e. 1 whole token) to [mint] cTokens
  • They will get back some underlying token amount * [initialExchangeRateMantissa] of cTokens because of totalSupply is zero.
  • They go head and redeem all cTokens but they got 1 wei.
  • They transfer z underlying tokens directly to cToken address - This leads to 1 wei of cToken worth z (+ some small amount) - Attacker won't have any problem making this z as big as possible as they have all the claim to it as a holder of 1 Wei of cToken.

Tools Used

Code Review

Recommended Mitigation Steps

For new cTokens, I like how BalancerV2 and [UniswapV2] (https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L119-L121) do it.

Some minimum amount of pool tokens get burnt when the first mint happens.

Activity

  1. added
    3 (High Risk)Assets can be stolen/lost/compromised directly
    bugSomething isn't working
    on Jan 15, 2023
  2. added a commit that references this issue on Jan 15, 2023
  3. added
    QA (Quality Assurance)Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
    and removed
    3 (High Risk)Assets can be stolen/lost/compromised directly
    on Jan 22, 2023
  4. c4-judge commented on Jan 22, 2023

    @c4-judge
    Contributor

    trust1995 changed the severity to QA (Quality Assurance)

  5. c4-judge commented on Jan 22, 2023

    @c4-judge
    Contributor

    trust1995 marked the issue as grade-b

  6. ypatil12 commented on Jan 24, 2023

    @ypatil12

    Should be a duplicate with: #247

  7. added and removed on Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Q-43QA (Quality Assurance)Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntaxbugSomething isn't workingdowngraded by judgeJudge downgraded the risk level of this issuegrade-b

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions