Regular bonding incorrectly purchases a reduced amount of Put options due to discount

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L904-L924

Vulnerability details

During rDPX bonding, rdpxV2Core will purchase put options for the rDPX that are used for minting DPXETH. This is for hedging against rDPX price drop to protect the DPXETH peg. The amount of put options to purchase is equivalent to the amount of rDPX that are used for minting DPXETH, which is rdxRequired as given by calculateBondCost().

The issue is that for regular bonding, rdxRequired is reduced by the discount provided by Treasury Reserve and the amount of rDPX used for minting is actually rdxRequired + rDPX discount given by Treasury Reserve. We can see the _transfer() code below actually increment the rDPX Backing Reserve by _rdpxAmount + extraRdpxToWithdraw, which is the actual amount of rDPX use for minting DPXETH.

So users that perform regular/delegate bonding will pay less premium due to the reduced amount of put options. With a reduced amount of put options, the hedging in place to protect DPXETH peg will be less effective.

Impact

Users who performs bonding via rDPX Decaying Bond will be shortchanged as they are paying the full premium while users who perform regular/delegate bonding will pay less premium than required.

Also, with a reduced amount of put options, it will reduce effectiveness of the hedging against rDPX price drop, thereby leading to higher likelihood of DPX ETH depeg.

Proof of Concept

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L904-L924

  function bond(
    uint256 _amount,
    uint256 rdpxBondId,
    address _to
  ) public returns (uint256 receiptTokenAmount) {
    _whenNotPaused();
    // Validate amount
    _validate(_amount > 0, 4);

    //@audit rdpxRequired is reduced by discount for regular bonding
    // Compute the bond cost
    (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
      _amount,
      rdpxBondId
    );

    //@audit for regular bonding, it should purchase rdpxRequired + rdpx discount
    // purchase options
    uint256 premium;
    if (putOptionsRequired) {
      premium = _purchaseOptions(rdpxRequired);
    }

    //@audit we can see the _transfer() code below actually uses _rdpxAmount + extraRdpxToWithdraw
     _transfer(rdpxRequired, wethRequired - premium, _amount, rdpxBondId);

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1162-L1178

  function calculateBondCost(
    uint256 _amount,
    uint256 _rdpxBondId
  ) public view returns (uint256 rdpxRequired, uint256 wethRequired) {
    uint256 rdpxPrice = getRdpxPrice();

    if (_rdpxBondId == 0) {
      uint256 bondDiscount = (bondDiscountFactor *
        Math.sqrt(IRdpxReserve(addresses.rdpxReserve).rdpxReserve()) *
        1e2) / (Math.sqrt(1e18)); // 1e8 precision

      _validate(bondDiscount < 100e8, 14);

      //@audit for regular bonding rdpxRequired is reduced by discount
      rdpxRequired =
        ((RDPX_RATIO_PERCENTAGE - (bondDiscount / 2)) *
          _amount *
          DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L681-L684
  function _transfer(
    uint256 _rdpxAmount,
    uint256 _wethAmount,
    uint256 _bondAmount,
    uint256 _bondId
  ) internal {

    //@audit amount of rDPX used for minting is _rdpxAmount + extraRdpxToWithdraw due to discount
      reserveAsset[reservesIndex["RDPX"]].tokenBalance +=
        _rdpxAmount +
        extraRdpxToWithdraw;
    }

Recommended Mitigation Steps

Update _purchaseOptions(rdpxRequired) to include the discount provided by Treasury Reserve.

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[bytes032]

Related to #1921

[c4-sponsor]

psytama (sponsor) disputed

[psytama]

This is a design choice and the code works as intended.

[GalloDaSballo]

POC from the Warden:

The following POC provides the details to shows the validity of the issue and the code is not working correctly as mentioned above.

1. First, to show premium paid, add the following console.log to RdpxV2Core.sol#L922

     console.log("premium paid = %d", premium);

2. For the POC, add and run the following code in tests\rdpxV2-core\Integration.t.sol

  function testPeakboltIncorrectOptionPurchase() public {

    console.log("------------- bonding with decaying bond  (1 dpxETH)-------------");

    // test with rdpx decaying bonds
    rdpxDecayingBonds.grantRole(rdpxDecayingBonds.MINTER_ROLE(), address(this));
    rdpxDecayingBonds.mint(address(this), block.timestamp + 10, 125 * 1e16);
    assertEq(rdpxDecayingBonds.ownerOf(1), address(this));
    rdpxDecayingBonds.approve(address(rdpxV2Core), 1);

    (uint256 rdpxRequiredToBond, ) = rdpxV2Core.calculateBondCost(1 * 1e18, 1);
    console.log("rdpxRequiredToBond: %d", rdpxRequiredToBond);

    // bond using decaying rdpxBondId 1 
    uint256 wethBalanceBefore = weth.balanceOf(address(this));
    (, uint256 rdpxReserveBefore, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    rdpxV2Core.bond(1 * 1e18, 1, address(this));

    uint256 wethBalanceAfter = weth.balanceOf(address(this));
    uint256 wethPaidForDecayingBonding = wethBalanceBefore - wethBalanceAfter;
    (, uint256 rdpxReserveAfter, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    uint256 rdpxReserveChange = rdpxReserveAfter - rdpxReserveBefore;
    console.log("rDPX reserve increase: %d", rdpxReserveChange);

    uint256 optionsIndex = vault.balanceOf(address(rdpxV2Core)) - 1;
    (uint256 strike, uint256 optionsAmount, ) = vault.optionPositions(optionsIndex);
    console.log("option purchased for rdpxRequired: %d", optionsAmount);

    // this will pass as amount of option purchase is equal to rdpx amount contributed to reserve
    assertEq(optionsAmount, rdpxReserveChange);

    console.log("------------- regular bonding (1 dpxETH) -------------");
    (rdpxRequiredToBond, ) = rdpxV2Core.calculateBondCost(1 * 1e18, 0);
    console.log("rdpxRequiredToBond: %d ", rdpxRequiredToBond);

    // regular bonding
    wethBalanceBefore = weth.balanceOf(address(this));
    (, rdpxReserveBefore, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    rdpxV2Core.bond(1 * 1e18, 0, address(this));

    wethBalanceAfter = weth.balanceOf(address(this));
    uint256 wethPaidForRegularBonding = wethBalanceBefore - wethBalanceAfter;

    (, rdpxReserveAfter, ) = rdpxV2Core.getReserveTokenInfo("RDPX");
    rdpxReserveChange = rdpxReserveAfter - rdpxReserveBefore;
    console.log("rDPX reserve increase: %d", rdpxReserveChange);

    optionsIndex = vault.balanceOf(address(rdpxV2Core)) - 1;
    (strike, optionsAmount, ) = vault.optionPositions(optionsIndex);
    console.log("option purchased for rdpxRequired: %d", optionsAmount);

     // this will fail as amount of option purchased < rdpx amount contributed to reserve (incorrect)
     // that means rdpxV2Core has a smaller than required hedge against rDPX price drop, which is wrong
     assertEq(optionsAmount, rdpxReserveChange);

    // this will fail as users performing regular bonding is paying less weth than users bonding with decaying bond
    // this is due to a lower premium (incorrect) paid for regular bonding, because of incorrect option purchased 
    assertEq(wethPaidForDecayingBonding, wethPaidForRegularBonding);

  }

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[GalloDaSballo]

I've adjusted the POC to revert after the first operations as to not change the balance of reserves which has been demonstrated to change pricing

The output

  ------------- bonding with decaying bond  (1 dpxETH)-------------
  rdpxRequiredToBond: 1250000000000000000
  rDPX reserve increase: 1250000000000000000
  option purchased for rdpxRequired: 1250000000000000000
  ------------- regular bonding (1 dpxETH) -------------
  rdpxRequiredToBond: 1225000000000000000 
  rDPX reserve increase: 1275000000000000000
  option purchased for rdpxRequired: 1225000000000000000
  Error: a == b not satisfied [uint]
        Left: 1225000000000000000
       Right: 1275000000000000000
  Error: a == b not satisfied [uint]
        Left: 812500000000000000
       Right: 806250000000000000

Unless I'm missing something, the Warden has shown how, by purchasing effectively the same thing, we can get two difference prices

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[c4-judge]

GalloDaSballo marked the issue as selected for report

[c4-judge]

GalloDaSballo marked issue #780 as primary and marked this issue as a duplicate of 780