sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

CL001 - Users can avoid paying borrowing fees #233

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

CL001

Medium

Users can avoid paying borrowing fees

Summary

realizedBorrowingFeeDelta is used to determine the amount of realizedBorrowingFee base on the current Position initialMargin and leverage

However, the borrowing fee is not updated when the position leverage is changed https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/PositionMarginProcess.sol#L134

Vulnerability Detail

Consider the following scenario: To avoid paying the borrowing fee, the user place an order with 1x leverage, then uses the executeUpdateLeverageRequest function to change the leverage.

1.user0 place order 1: https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/IncreasePositionProcess.sol#L84 orderMargin=0.1ETH , leverage =10000, realizedBorrowingFee =0 time.increase(199)

2.user0 place order 2: https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/IncreasePositionProcess.sol#L84 orderMargin=0.1ETH , leverage =10000, realizedBorrowingFee =0

3.user0 update leverage: https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/facets/PositionFacet.sol#L108 leverage =800000 realizedBorrowingFee =0

POC:

import { expect } from 'chai'
import { Fixture, deployFixture } from '@test/deployFixture'
import { ORDER_ID_KEY, OrderSide, OrderType, PositionSide, StopType } from '@utils/constants'
import { precision } from '@utils/precision'
import { deposit } from '@utils/deposit'
import {
  AccountFacet,
  FeeFacet,
  MarketFacet,
  MockToken,
  OrderFacet,
  ConfigFacet,
  PoolFacet,
  PositionFacet,
  TradeVault,
} from 'types'
import { HardhatEthersSigner } from '@nomicfoundation/hardhat-ethers/signers'
import { time } from "@nomicfoundation/hardhat-network-helpers";
import { ethers } from 'hardhat'
import { Contract } from 'ethers'
import { handleOrder } from '@utils/order'
import { handleMint } from '@utils/mint'
import { account } from '@utils/account'
import { pool } from '@utils/pool'
import { oracles } from '@utils/oracles'
import { handleUpdateLeverage, handleUpdateMargin } from '@utils/position'

describe(' Order Process', function () {
    let fixture: Fixture
    let tradeVault: TradeVault,
      marketFacet: MarketFacet,
      orderFacet: OrderFacet,
      poolFacet: PoolFacet,
      accountFacet: AccountFacet,
      positionFacet: PositionFacet,
      feeFacet: FeeFacet,
      configFacet: ConfigFacet
    let user0: HardhatEthersSigner, user1: HardhatEthersSigner, user2: HardhatEthersSigner, user3: HardhatEthersSigner
    let diamondAddr: string,
      tradeVaultAddr: string,
      wbtcAddr: string,
      wethAddr: string,
      solAddr: string,
      usdcAddr: string
    let btcUsd: string, ethUsd: string, solUsd: string, xBtc: string, xEth: string, xUsd: string
    let wbtc: MockToken, weth: MockToken, sol: MockToken, usdc: MockToken

    beforeEach(async () => {
      fixture = await deployFixture()
      ;({ tradeVault, marketFacet, poolFacet, orderFacet, configFacet, accountFacet, positionFacet, feeFacet } =
        fixture.contracts)
      ;({ user0, user1, user2, user3 } = fixture.accounts)
      ;({ btcUsd, ethUsd, solUsd } = fixture.symbols)
      ;({ xBtc, xEth, xUsd } = fixture.pools)
      ;({ wbtc, weth, sol, usdc } = fixture.tokens)
      ;({ diamondAddr, tradeVaultAddr } = fixture.addresses)
      wbtcAddr = await wbtc.getAddress()
      wethAddr = await weth.getAddress()
      solAddr = await sol.getAddress()
      usdcAddr = await usdc.getAddress()

      const btcTokenPrice = precision.price(25000)
      const btcOracle = [{ token: wbtcAddr, minPrice: btcTokenPrice, maxPrice: btcTokenPrice }]
      await handleMint(fixture, {
        stakeToken: xBtc,
        requestToken: wbtc,
        requestTokenAmount: precision.token(100),
        oracle: btcOracle,
      })

      const ethTokenPrice = precision.price(1600)
      const ethOracle = [{ token: wethAddr, minPrice: ethTokenPrice, maxPrice: ethTokenPrice }]
      await handleMint(fixture, {
        requestTokenAmount: precision.token(500),
        oracle: ethOracle,
      })

      const usdtTokenPrice = precision.price(1)
      const usdcTokenPrice = precision.price(101, 6)
      const daiTokenPrice = precision.price(99, 7)
      const usdOracle = [
        { token: usdcAddr, minPrice: usdcTokenPrice, maxPrice: usdcTokenPrice },
      ]

      await handleMint(fixture, {
        requestTokenAmount: precision.token(100000, 6),
        stakeToken: xUsd,
        requestToken: usdc,
        oracle: usdOracle,
      })

    })

    it('poc', async function () {
      const orderMargin1 = precision.token(1, 17) // 0.1ETH
      const ethPrice0 = precision.price(1800)
      const usdcPrice = precision.price(99, 6)
      const ethOracle0 = [{ token: wethAddr, minPrice: ethPrice0, maxPrice: ethPrice0 },
        { token: usdcAddr, targetToken: ethers.ZeroAddress, minPrice: usdcPrice, maxPrice: usdcPrice }]

      await handleOrder(fixture, {
        orderMargin: orderMargin1,
        oracle: ethOracle0,
        leverage: precision.rate(1),
      })

      const defaultMarginMode = false

      time.increase(199);

      const position1 = await positionFacet.getSinglePosition(user0.address, ethUsd, wethAddr, defaultMarginMode)
     console.log("realizedBorrowingFee, after handle Order 1: ",position1.positionFee.realizedBorrowingFee)
    //  console.log("realizedFundingFee1",position1.positionFee.realizedFundingFee)
     const orderMargin2 = precision.token(1, 17) // 0.1ETH
     const usdcAmount = precision.token(2000, 6)

     await handleOrder(fixture, {
      orderMargin: orderMargin2,
      oracle: ethOracle0,
      leverage: precision.rate(1),

    })
    const position2 = await positionFacet.getSinglePosition(user0.address, ethUsd, wethAddr, defaultMarginMode)
    console.log("realizedBorrowingFee ,after handle Order 2: ",position2.positionFee.realizedBorrowingFee)

    time.increase(199);
     // 
     const ethPrice1 = precision.price(1789)
     const ethOracle1 = [{ token: wethAddr, minPrice: ethPrice1, maxPrice: ethPrice1 }]
     const addMargin = precision.token(3, 16) //0.03ETH
     const executionFee = precision.token(2, 15)

      await handleUpdateLeverage(fixture, {
        symbol: ethUsd,
        isLong: true,
        isCrossMargin: defaultMarginMode,
        leverage: precision.rate(8), // 8 X leverage
        addMarginAmount: addMargin,
        marginToken: weth,
        oracle: ethOracle1,
        executionFee: executionFee,
      })
      const position3 = await positionFacet.getSinglePosition(user0.address, ethUsd, wethAddr, defaultMarginMode)
    console.log("realizedBorrowingFee ,after Update Leverage : ",position3.positionFee.realizedBorrowingFee)
    console.log("position leverage,after Update Leverage : ",position3.leverage)
    expect(position3.leverage).to.equals(precision.rate(8))

    })

  })

realizedBorrowingFee, after handle Order 1: 0n realizedBorrowingFee ,after handle Order 2: 0n realizedBorrowingFee ,after Update Leverage : 0n position leverage,after Update Leverage : 800000n

Impact

Users can avoid paying borrowing fees

Code Snippet

Tool used

Manual Review

Recommendation

update Position Leverage should take into account borrowing fee

Duplicate of #63

sherlock-admin2 commented 3 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/0xCedar/elfi-perp-contracts/pull/24

sherlock-admin2 commented 2 months ago

The Lead Senior Watson signed off on the fix.