sherlock-audit / 2023-09-ajna-judging

7 stars 6 forks source link

hyh - Reserves can be stolen by settling artificially created bad debt from them #71

Open sherlock-admin2 opened 1 year ago

sherlock-admin2 commented 1 year ago

hyh

high

Reserves can be stolen by settling artificially created bad debt from them

Summary

It is possible to inflate LUP and pull almost all the collateral from the loan, then settle this artificial bad debt off reserves, stealing all of them by repeating the attack.

Vulnerability Detail

LUP is determined just in time of the execution, but removeQuoteToken() will check for LUP > HTP = Loans.getMax(loans).thresholdPrice, so the attack need to circumvent this.

Attacker acts via 2 accounts under control, one for lending part, one for borrowing, having no previous positions in the given ERC20 pool. Let's say that pool state is as follows: 100 units of quote token utilized, and another 100 not utilized, for simplicity let's say all 100 are available, no auctions and no bad debt.

  1. Get 100 loan at TP = new HTP just below LUP (let's name this loan the manipulated_loan, ML), let's say LUP is such that 150 quote token units worth of collateral was provided (at market price)
  2. Add quote funds with amount = {total utilized deposits} = 200 to a bucket significantly above the market (let's name it ultra_high_bucket, UHB)
  3. Remove collateral from the ML up to allowed by elevated LUP = UHB price, say it's 10x market one. I.e. almost all, say remove 140, leaving 10 quote token units worth of collateral (at market price)
  4. Lender kick ML (removing funds from UHB will push LUP low and lender kick will be possible) to revive HTP reading
  5. Remove all from UHB except quote funds frozen by the auction, i.e. remove 100, leave 100

1-5 go atomically in one tx.

No outside actors will be able to immediately benefit from the resulting situation as: a. UHB remove quote token is blocked due to the frozen debt b. take is unprofitable while auction price is above market c. bucket take is unprofitable while auction price is above UHB price

At this point attacker accounting in net quote tokens worth is:

  1. +100, -150
  2. -200
  3. +140
  4. +100

Attacker has net 10 units of own capital still invested in the pool.

  1. Attacker wait for auction price reaching UHB price, calls takeBucket with depositTake == true. The call can go with normal gas price as outside actors will not benefit from the such call and there will be no competition. 10 quote tokens worth of collateral will be placed to UHB and 98.482 units of quote token removed to cover the debt, 1.518 is the kicker's reward
  2. Attacker calls settlePoolDebt() which settles the remaining 1.518 of debt from pool reserves as ML has this amount of debt and no collateral
  3. Attacker removes all the funds from UHB with both initial and awarded LP shares, receiving 10 quote tokens worth of collateral and 1.518 quote token units of profit

6-8 go atomically in one tx.

At this point attacker accounting in quote tokens is:

  1. (+kicker reward of ML stamped NP vs auction clearing UHB price in LP form)
  2. +11.518

Attacker has net 0 units of own capital still invested in the pool, and receives 1.518 quote token units of profit.

Profit part is a function of the pool's state, for the number above, assuming auctionPrice == thresholdPrice = UHB price and poolRate_ = 0.05: 1.518 = bondFactor * 100 = (npTpRatio - 1) / 10 * 100 = (1.04 + math.sqrt(0.05) / 2 - 1) / 10 * 100, bpf = bondFactor = takePenaltyFactor.

Impact

It can be repeated to drain the whole reserves from the pool over time, i.e. reserves of any pool can be stolen this way.

Code Snippet

LUP is the only guardian for removing the collateral:

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/libraries/external/BorrowerActions.sol#L287-L307

        if (vars.pull) {
            // only intended recipient can pull collateral
            if (borrowerAddress_ != msg.sender) revert BorrowerNotSender();

            // calculate LUP only if it wasn't calculated in repay action
            if (!vars.repay) result_.newLup = Deposits.getLup(deposits_, result_.poolDebt);

>>          uint256 encumberedCollateral = Maths.wdiv(vars.borrowerDebt, result_.newLup);
            if (
                borrower.t0Debt != 0 && encumberedCollateral == 0 || // case when small amount of debt at a high LUP results in encumbered collateral calculated as 0
                borrower.collateral < encumberedCollateral ||
                borrower.collateral - encumberedCollateral < collateralAmountToPull_
            ) revert InsufficientCollateral();

            // stamp borrower Np to Tp ratio when pull collateral action
            vars.stampNpTpRatio = true;

            borrower.collateral -= collateralAmountToPull_;

            result_.poolCollateral -= collateralAmountToPull_;
        }

The root cause is that bad debt is artificially created, with lender, borrower and kicker being controlled by the attacker, bad debt is then settled from the reserves:

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/libraries/external/SettlerActions.sol#L143-L146

            // settle debt from reserves (assets - liabilities) if reserves positive, round reserves down however
            if (assets > liabilities) {
                borrower.t0Debt -= Maths.min(borrower.t0Debt, Maths.floorWdiv(assets - liabilities, poolState_.inflator));
            }

Kicking will revive the HTP as _kick() removing target borrower from loans:

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/libraries/external/KickerActions.sol#L340-L341

        // remove kicked loan from heap
        Loans.remove(loans_, borrowerAddress_, loans_.indices[borrowerAddress_]);

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/base/Pool.sol#L225-L249

    function removeQuoteToken(
        ...
    ) external override nonReentrant returns (uint256 removedAmount_, uint256 redeemedLP_) {
        ...

        uint256 newLup;
        (
            removedAmount_,
            redeemedLP_,
            newLup
        ) = LenderActions.removeQuoteToken(
            ...
            RemoveQuoteParams({
                maxAmount:      Maths.min(maxAmount_, _availableQuoteToken()),
                index:          index_,
>>              thresholdPrice: Loans.getMax(loans).thresholdPrice
            })
        );

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L434-L436

        lup_ = Deposits.getLup(deposits_, poolState_.debt);

>>      uint256 htp = Maths.wmul(params_.thresholdPrice, poolState_.inflator);

Tool used

Manual Review

Recommendation

Consider introducing a buffer representing the expected kicker reward in addition to LUP, so this part of the loan will remain in the pool.

kadenzipfel commented 1 year ago

Escalate Should be medium.

According to the judging docs:

The profit margin from the provided example is ~0.4% (1.518 / (150 quote tokens worth of collateral at ML + 200 quote tokens at UHB) * 100%). This is consistent with the medium issue criteria ("minimal profit").

The minimum cost of the attack is greater than the total utilized deposits of the pool (Note: it's noted that the UHB is funded with the total utilized deposits of the pool, but the actual amount used in the example is 200, 2x the total utilized deposits and actually equal to the total utilized and unutilized deposits of the pool). Also note that it's not possible to use a flashloan in this circumstance since the attack requires multiple transactions across two EOAs. This is also consistent with the medium issue criteria ("considerable cost").

sherlock-admin2 commented 1 year ago

Escalate Should be medium.

According to the judging docs:

The profit margin from the provided example is ~0.4% (1.518 / (150 quote tokens worth of collateral at ML + 200 quote tokens at UHB) * 100%). This is consistent with the medium issue criteria ("minimal profit").

The minimum cost of the attack is greater than the total utilized deposits of the pool (Note: it's noted that the UHB is funded with the total utilized deposits of the pool, but the actual amount used in the example is 200, 2x the total utilized deposits and actually equal to the total utilized and unutilized deposits of the pool). Also note that it's not possible to use a flashloan in this circumstance since the attack requires multiple transactions across two EOAs. This is also consistent with the medium issue criteria ("considerable cost").

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

neeksec commented 1 year ago

Agree with Escalation to downgrade severity to Medium.

dmitriia commented 1 year ago

The use of flash loans is possible as borrower and lender can have the same address of the attacking smart contract, there is no restrictions in this regard. 0.4% is the immediate, atomic profit margin. The funds that are required to stay in the contract for auction are 10, not 200. If flash loans are free, for example obtained from Ajna itself, where the fee is zero, the total profit for the attacker in abstract quote token units is 1.518 - {2 days funding for 10} - total gas.

This can be quite significant, say if 1 unit here is USDC 10k, the profit is 15.18k - USDC_rate*2.0/365 * 100k - gas. If we assume that USDC borrow rate is 10% and total gas is 1 ETH = USD 2k, it becomes USDC 15.18 - 0.1*2.0/365*100 - 2 = 13.12k. There is basically no market risk involved here, so it's arbitrage profit significant enough to compensate attacker for the efforts. This is for one pool and the attacking contract can be immediately reused for all the others.

As reserves of any pool can be drained this way fully, the reserves from all pools on all chains can be stolen. Since reserves burning is core monetization mechanics for protocl token this basically means that Ajna token will be worthless once this vector becomes public. Also, lack of reserves over time will move the growing share of pools to be insolvent as reserves also serve for bad debt coverage. Insolvency will render the pools unusable. The actual severity here is critical, but given the lack of this grade it is marked as high.

kadenzipfel commented 1 year ago

0.4% is the immediate, atomic profit margin.

No. As you explained in your example, steps 1-5 are atomic and by the end of step 5 the attacker is at a loss of 10 units of collateral. Profit doesn't occur until the attacker completes steps 6-8, and only if auction price gets low enough for attacker to execute these steps.

There is basically no market risk involved here

This is false. The attacker is at a 10 unit loss (10% of the entire total utilized deposits) for, by your estimation, 2 days. They only ever profit if the auction price reaches the UHB. The Ajna team thus has 2 days to assess a highly suspicious auction and prevent further losses, in which they can simply take the quote tokens before the attacker, griefing the attacker into a significant loss and disincentivizing the attack entirely.

In summary, regardless of flashloans, as noted in escalation:

kadenzipfel commented 1 year ago

This issue should also be invalid until a valid proof of concept is provided considering this attack is highly complex and far from obvious. See: In case of non-obvious issues with complex vulnerabilities/attack paths, Watson must submit a valid POC for the issue to be considered valid and rewarded.

dmitriia commented 1 year ago

Market risk is risk that stems from movements of the market, it is absent in this scenario. Ajna team has little means to prevent the attack as the protocol is permissionless, while take() requires paying for collateral at prices substantially above the market, i.e. it is a material external investment that will be needed for all the pools. Also, if anyone takes before the attacker it will benefit them substantially (they are also a borrower, which will have their collateral priced even higher than they manipulated it themselves), resulting in even higher profitability, which will not disincentivize the attack.

Again, this is critical severity vulnerability, having no low probability prerequisites and inflicting direct loss for the Ajna holders and pool's depositors (when reserves are lacking bad debt is written off from user's deposits starting with HPB).

dmitriia commented 1 year ago

This issue should also be invalid until a valid proof of concept is provided considering this attack is highly complex and far from obvious

Protocol team has already run PoC for this, @ith-harvey, please confirm.

Czar102 commented 12 months ago

From my understanding, this bug makes it possible for the attacker to immediately create bad debt. Even if the earnings/losses are a small fraction of the whole TVL of the pool, it is extremely severe for a lending protocol. Planning to keep it a high severity issue unless I'm misinterpreting or missing something.

Czar102 commented 12 months ago

Result: High Unique

sherlock-admin2 commented 12 months ago

Escalations have been resolved successfully!

Escalation status:

ith-harvey commented 11 months ago

The following code changes resolve this issue:

dmitriia commented 11 months ago

Fix via PRs 962 and 1008 looks ok, conditional on the protocol running keeper bot that performs settlement calls.

The attack described can still be successful when there is a lack of activity in the pool, i.e. if we suppose that there is no party besides the attacker who will run the settlement and force attacker's deposit position to cover for the attack induced bad debt, then the attack still can be carried out with profit. When settlement does happen the attack provides no profit, but since there is also no principal fund loss for the attacker and cost of attack is substantially lower than potential profit, it might be the case that for low activity pools there still be some attempts to hold the position for 144 hours and close bad debt off the reserves fully. In order to remove such a possibility opting for an automated solution that runs settlement whenever market participants didn't during a substantial period for any reason, i.e. protocol team controlled settlement of the last resort, is advised.

jacksanford1 commented 11 months ago

Sherlock notes that this issue is fixed, but it's contingent on Ajna ensuring a keeper bot runs to perform settlement calls.

ith-harvey commented 10 months ago

Sherlock notes that this issue is fixed, but it's contingent on Ajna ensuring a keeper bot runs to perform settlement calls.

We don't believe that the resolution to this issue should require or mention the maintenance of a keeper to ensure loans are settled for the following reasons:

Ajna is designed to be fully decentralized, and there will not be a centralized keeper running, and we'd like the report to reflect that.