Dopex - tapir'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: 35/189

Findings: 10

Award: $571.11

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

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/perp-vault/PerpetualAtlanticVault.sol#L315-L368 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L764-L783

Vulnerability details

Impact

When Core contract buys put options while executing the bond requests from users it will interact with the APP contracts purchase function. Bought option NFT's are then sent to Core contract and can be settled by admin of the Core contract. When settlement happens APP contract's "settle" function will be called. Inside the function body APP contract calls the vaultLP contracts "substractLoss" function which is relying on ERC20 balanceOf function to validate the state. This function can easily be manipulated and make bonding process revert forever.

Proof of Concept

Assume that the Core contract has some options that can be exercised. Admin will call the core contracts settle function and the settlement process will begin. Also, assume that just before this function vaultLP contract has: 10WETH == balanceOf(WETH) == _totalActiveCollateral 100rDPX == balanceOf(rDPX) == _rdpxCollateral

Now, just before the settlement a malicious user transfers 1 wei WETH and 1 wei rDPX to vault LP contract. This will make the balances of the vaultLP contract as follows: _totalActiveCollateral = 10WETH _rdpxCollateral = 100rDPX balanceOf(WETH) = 10WETH + 1 balanceOf(rDPX) = 100rDPX + 1

Now, assume that the "ethAmount" = 1WETH and "rdpxAmount" = 10rDPX. It means that 1WETH will be transferred from the vaultLP contract and 10rDPX will be transferred to the vaultLP contract. After the transfers the balance of the vaultLP contract as follows: _totalActiveCollateral = 10WETH _rdpxCollateral = 100rDPX balanceOf(WETH) = 9WETH + 1 balanceOf(rDPX) = 110rDPX + 1

When the substractLoss function is called this will be the checked inside the vault LP contract: collateral.balanceOf(address(this)) == _totalCollateral - loss (9WETH + 1) == 10WETH - 1WETH == 9WETH + 1 != 9WETH hence the function will revert.

To conclude, anytime a direct ERC20 collateral token transfer happens to vault LP contract, the settlement will never go through because of the require check here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L201

Since the attack is very easy to do and it bricks the entire settlement process I'll label this as high

Tools Used

Manual

Do not rely on balanceOf function when accounting the collateral balance in the vault LP contract.

Assessed type

ERC20

#0 - c4-pre-sort

2023-09-13T06:33:27Z

bytes032 marked the issue as duplicate of #619

#1 - c4-pre-sort

2023-09-13T06:33:31Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-20T19:35:22Z

GalloDaSballo marked the issue as satisfactory

Findings Information

Awards

181.367 USDC - $181.37

Labels

bug
3 (High Risk)
satisfactory
duplicate-935

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L324-L336

Vulnerability details

Impact

UniswapV3AMO holds Uniswap LP positions of the assets its responsible of. Admin role can use recover function to send any NFT to Core contract. However, core contract can not handle the NFT that it received. Core contract has no functionality to transfer the NFT back to somewhere or do any Uniswap actions like collect-remove-mint. In the case of admin calls recover and send it to Core contract, the NFT will be unusable forever.

Proof of Concept

Assume the UniswapV3AMO contract has an LP position of rDPX-WETH 1% pool. In emergency admin decides to recover all the NFT's by calling this function https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L324-L336

Now, the NFT's are in the rdpxCore contract. However, they will be stuck in the Core contract hence the protocol loses a major liquidity source.

Considering the NFT can not be handled anymore it will break the core functionality of the AMO. I will classify this as a high finding.

Tools Used

Manual

Add functionality to handle Uniswap NFT's (like a simple NFT transfer for general) to handle the situation where AMO sends the NFT's to Core contract.

Assessed type

Uniswap

#0 - c4-pre-sort

2023-09-12T06:11:35Z

bytes032 marked the issue as duplicate of #935

#1 - c4-judge

2023-10-20T18:05:56Z

GalloDaSballo marked the issue as satisfactory

Awards

17.313 USDC - $17.31

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
duplicate-867

External Links

Lines of code

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

Vulnerability details

Impact

When an user deposits to the vault lp the vault lp first calculates the shares to mint and then calls the updateFunding() function from the APP contract. This function will send the funding accrued from the last time its called to the vault lp contract hence, the total assets in the vault lp contract will increase. If an user deposits and immediately withdraws, the user will have more collateral token then he had before deposit. User can do this action in 1 tx to atomically cash out some of the funding risk free.

Proof of Concept

Assume the next time updateFunding() will be called in APP contract will accrue 0.001WETH to the vault lp contract. Also, assume that the vaultLP has 100 supply and 100 collateral balance. When an user deposits 10WETH to vault lp contract the shares will be calculated as 10 * 100/100 = 10 which means that the user will get 10 vault lp shares. After the previewDeposit the accrued WETH will be transferred from the APP contract to vault lp contract as seen here https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L125 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L502-L524 Which will increase the total collateral amount to 100.001WETH.

Now, if the user immediately redeems the 10 vault shares, user will get more than 10WETH in exchange because of the accrued WETH. Redeem call will first accrue an another funding but since the user calls it in same tx the block.timestamp will be same with the latest funding accrue time hence, this will not accrue any WETH. Then, redeemPreview will be executed which will call _convertToAssets, as we can see in the convertToAssets function the shares will be calculated as follows: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L218-L229 10 * (100 + 0.001) / 100 = 10.0001WETH which then will be sent to user. In result, user cashed out 0.0001WETH instantly without taking any risk in only 1 tx.

This is a logic error and it can affect the protocol badly especially if the fundings accrued is very high. I'll label this as high.

Tools Used

Manual

First accrue the fundings and then call the previewDeposit to get the latest collateral balance in the contract.

Assessed type

ERC4626

#0 - c4-pre-sort

2023-09-07T13:31:58Z

bytes032 marked the issue as duplicate of #1948

#1 - c4-pre-sort

2023-09-07T13:42:23Z

bytes032 marked the issue as duplicate of #867

#2 - c4-pre-sort

2023-09-11T09:04:12Z

bytes032 marked the issue as sufficient quality report

#3 - c4-judge

2023-10-20T19:26:21Z

GalloDaSballo marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L941-L968 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L975-L990

Vulnerability details

Impact

When users add WETH to core contract for delegation they can withdraw the WETH as long as it is not bonded. When delegation happens the storage variable "totalWethDelegated" is incremented and it plays a significant role for Core contracts WETH balance tracking. However, when WETH is withdrawn via withdraw function the same "totalWethDelegated" is not decremented which can cause big accounting problems for Core contract.

Proof of Concept

Assume the core contract has 7WETH in its reserves, reserveAsset[reservesIndex["WETH"]].tokenBalance = 7WETH. Assume an user calls "addToDelegate" and delegates 10WETH to Core contract. This function will increment "totalWethDelegated" +10. At this stage, core contract has 10WETH as delegated and 7WETH in reserves. Right after the adding WETH tx, same user calls "withdraw" function and receives back the 10WETH. However, "totalWethDelegated" is still 10. This means that anything that relies on Core contracts "sync()" function will revert because of the underflow as we can see here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L995-L1008 For example, reLP contract will not be able to be called because it calls Core contracts sync() function as seen here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L306

Since reLP'ing is very important for the Core contract and reLP'ing will be impossible to call the "isReLPActive" flag has to be false. If the flag is set to true then the bonding will not go through which is the main function for Core contract. Hence, I classified this as a high finding

Tools Used

Manual

Decrement "totalWethDelegated" variable when users withdraws delegated WETH.

Assessed type

Other

#0 - c4-pre-sort

2023-09-07T07:58:01Z

bytes032 marked the issue as duplicate of #2186

#1 - c4-judge

2023-10-20T17:55:32Z

GalloDaSballo changed the severity to 2 (Med Risk)

#2 - c4-judge

2023-10-20T17:56:15Z

GalloDaSballo marked the issue as satisfactory

#3 - c4-judge

2023-10-21T07:38:54Z

GalloDaSballo changed the severity to 3 (High Risk)

#4 - c4-judge

2023-10-21T07:46:15Z

GalloDaSballo marked the issue as partial-50

Findings Information

Awards

39.433 USDC - $39.43

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sufficient quality report
duplicate-1805

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L202-L307 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L273-L284

Vulnerability details

Impact

reLP contract removes liquidity and receives rDPX and WETH. Half of the received WETH is swapped to rDPX and paired with the remaining WETH to add liquidity back again. When the swap happens from WETH-rDPX the minimum token amount A is calculated wrongly. The price used in the formula is the price for rDPX token in terms of ETH but to calculate the correct rDPX amount the opposite pricing is needed, price of 1WETH token in terms of rDPX.

Proof of Concept

The following code is executed when swapping the WETH to rDPX

                // calculate min amount of tokenA to be received
		mintokenAAmount =
			(((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
			(((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);

		uint256 tokenAAmountOut = IUniswapV2Router(addresses.ammRouter)
			.swapExactTokensForTokens(
				amountB / 2,
				mintokenAAmount,
				path,
				address(this),
				block.timestamp + 10
			)[path.length - 1];

to give more clarity, tokenA is rDPX and tokenB is WETH and tokenAPrice is 1 rDPX price in terms of WETH as we can see in here https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L204-L226

Let's assume: amountB = 1WETH tokenAPrice = 0.01WETH (1rDPX == 0.01WETH) slippageTolerance = 1%

mintokenAAmount = ((1 / 2) * 0.01) - ((1 / 2) * 0.01(price) * 0.01(slippage)) = 0.00495

as we can see the minimum token A amount is calculated as 0.00495 rDPX, which is wrong. If the price would be in terms of rDPX than we would get the correct value for the minTokenAAmount. 0.5WETH will be swapped to rDPX and the minimum rDPX amount is 0.00495. Which is way off considering 1WETH is 100rDPX.

When the min token amount is calculated mistakenly the removing liquidity and re-adding part can go immensely wrong! Here an example:

Assume pool has 10 WETH and 1000 rDPX and the total LP supply for the pool is 6. Assume that the reLP process needs to remove 2.41 LP, which would correspond to 4.01WETH and 401 rDPX. After the removed liquidity the pool now have 5.99WETH and 599rDPX. Now, the reLP contract will sell the half of the WETH for rDPX on that same pool.

UniswapV2 AMM 5.99 * 599 = k k = 3582

(5.99 + 2) * (599 - dx) = 3582 dx = 150.69

As we see that swapping 2WETH ended up with only 150rDPX although the price of rDPX was 0.01WETH in the oracle and would've been easily prevented if the minimumAmount calculation were correct. This would make AMO to swap inefficiently and damage the liquidity management of rDPX-WETH in the long run.

Tools Used

Manual, Desmos calculator for math

Modify the price variable. It should be the 1WETH worth of rDPX to get the desired value for the minimum token amount.

Assessed type

Uniswap

#0 - c4-pre-sort

2023-09-10T10:23:43Z

bytes032 marked the issue as duplicate of #1805

#1 - c4-pre-sort

2023-09-11T07:05:08Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-16T08:47:53Z

GalloDaSballo changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-10-20T09:26:38Z

GalloDaSballo marked the issue as satisfactory

Awards

46.2486 USDC - $46.25

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sufficient quality report
duplicate-863

External Links

Lines of code

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#L269-L284

Vulnerability details

Impact

When first deposit happens in the vault the shares that the depositor will get will be 1:1 with the assets thats been provided. If user deposits 1wei (very small amount) asset the totalSupply and total collateral will be 1. When the next user comes because of solidity math truncating the share to mint calculation will result on division with zero hence, the deposit will be impossible for the next users.

Proof of Concept

Assume that the vault just deployed and malicious user deposited 1 wei. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L269-L284 since totalSupply == 0 the user shares will be calculated as 1 wei. Now, totalSupply is 1 and _totalCollateral is also 1. Also, there are no rDPX collateral in the vault since its just deployed.

Next, an another user wants to deposit 10WETH to vault. When the same code lines execute in previewDeposit the shares will be calculated as follows: totalVaultCollateral = 1 / 1e8 = 0 in solidity However, since supply is not 0 anymore the second statement will be executed which will do this calculation: assets.mulDivDown(supply, totalVaultCollateral); Since totalVaultCollateral is 0, division with 0 occurs and function call will be reverted.

It is very easy to execute this attack and it will make vault deposits impossible. Hence, I'll label this as high.

Tools Used

Manual

Introduce an minimum initial deposit cap so that vault ensures the division error will not occurs.

Assessed type

ERC4626

#0 - c4-pre-sort

2023-09-07T13:32:08Z

bytes032 marked the issue as duplicate of #863

#1 - c4-pre-sort

2023-09-11T09:10:31Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-18T12:41:47Z

GalloDaSballo changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-10-18T12:53:25Z

GalloDaSballo marked the issue as satisfactory

Findings Information

🌟 Selected for report: gjaldon

Also found by: Evo, HChang26, QiuhaoLi, Toshii, Yanchuan, peakbolt, rvierdiiev, tapir

Labels

bug
2 (Med Risk)
satisfactory
duplicate-750

Awards

238.7514 USDC - $238.75

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#L218-L229 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L315-L344 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L764-L783

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

Assume the vaultLP contract has 10WETH and 0 rDPX and also assume that the core contract holds 5 positions of average strike price of 1rDPX = 0.02ETH. After sometime assume there are no deposits and withdrawals from the vaultLP contract and the price of 1rDPX = 0.01ETH. Now, Core contract can exercise the put options and receive WETH in exchange of rDPX. When Core contract calls settle it will trigger the APP contracts settle function. Inside the settlement function let's assume the total 'amount' is 500 rDPX. These lines will be calculated as follows:

ethAmount += (amount * strike) / 1e8;
rdpxAmount += amount; 

ethAmount = 500 * 0.02 = 10WETH rdpxAmount = 500 = 500 rDPX

2WETH will be transferred from the vaultLP contract to Core contract and Core contract will transfer 500 rDPX to vaultLP contract as compensation.

Now, the vaultLP contract holds 0WETH and 500rDPX. All WETH is exercised and sent to Core contract and everything works correctly so far. However, withdrawals from vaultLP contract is now impossible to go through because there are 0WETH. Let's check the withdrawal internal function here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L218-L229

Let's say an user wants to redeem 5 vault tokens for WETH+rDPX. According to above formula do calculation for assets will be as follows: 5 * (0 / 10) = 0 5 * (500/10) = 250rDPX

However, since assets is 0 the function will revert in this line of the redeem function: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L162

If someone does not deposit WETH to contract the vault share token holders will not be able to cash out their rDPX. If someone deposits 10WETH at this stage, the depositor will mint 20 shares and the previous withdrawal attempt would result in 1.6WETH + 83.33rDPX. Basically, the WETH deposited from the last user is socialized to other users in exchange for more portion of the rDPX for the last depositor. This could be an intended behaviour since vault depositors are exposed to both rDPX and WETH. However, if the assets are "0" then redeeming is impossible and someone needs to deposit some amount of WETH to surpass the require check above. Considering these, I classified this finding as medium.

Tools Used

If the assets are 0, check the rDPX amount. If that's also 0 then revert. If not just give the rDPX to user.

Assessed type

ERC4626

#0 - c4-pre-sort

2023-09-15T06:15:36Z

bytes032 marked the issue as duplicate of #750

#1 - c4-judge

2023-10-15T18:03:44Z

GalloDaSballo marked the issue as satisfactory

Awards

3.9186 USDC - $3.92

Labels

bug
2 (Med Risk)
downgraded by judge
partial-50
sufficient quality report
duplicate-269

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L160-L250

Vulnerability details

Impact

AMO contract can transfer "from" and "to" the Core contract. Core contract has its own storage balance sheet which is critic to issue bonds, funding fees and settlement. However, AMO contract uses ERC20 transfer functions hence the internal balance keeping is not updated inside the Core contract.

Proof of Concept

When AMO contract adds liquidity, AMO takes the tokens from the Core without updating the Core's internal storage balance keeping. Also, AMO does not update when sending tokens to Core contract via "_sendTokensToRdpxV2Core" internal function.

There are bunch of functions that rely on the internal storage balance keeping of the Core contract. The problematic ones would be the ones where the balance is decremented inside the function. Since the storage balances are not updated when AMO is called, these functions can have underflowing error that would make them impossible to call.

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L570

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L780

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L486

In addition to this underflow issue, since the WETH delegaters deposits WETH to the Core contract, the delegated WETH's can also be taken from the contract accidentally halting the delegation process.

Tools Used

Manual

Call Core contracts sync() function right after the AMO's functions. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L995-L1005

Segregate the delegated WETH inside the Core contract such that AMO can't take the delegated WETH balance of the Core contract.

Assessed type

Token-Transfer

#0 - c4-pre-sort

2023-09-09T03:48:51Z

bytes032 marked the issue as duplicate of #798

#1 - c4-pre-sort

2023-09-09T04:09:21Z

bytes032 marked the issue as duplicate of #269

#2 - c4-pre-sort

2023-09-11T11:58:24Z

bytes032 marked the issue as sufficient quality report

#3 - c4-judge

2023-10-15T18:13:07Z

GalloDaSballo marked the issue as not a duplicate

#4 - c4-judge

2023-10-20T07:46:48Z

GalloDaSballo marked the issue as duplicate of #250

#5 - c4-judge

2023-10-20T07:46:54Z

GalloDaSballo marked the issue as partial-50

#6 - GalloDaSballo

2023-10-20T07:47:04Z

Would benefit by a higher quality description

#7 - c4-judge

2023-10-20T19:39:05Z

GalloDaSballo changed the severity to 2 (Med Risk)

Awards

24.8267 USDC - $24.83

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
duplicate-153

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L202-L307

Vulnerability details

Impact

reLP contract can have WETH leftovers while adding liquidity. This amount can be a dust (very small) or can be a considerably big amount depending on the liquidity of the V2 pool and the "reLPFactor" variable. reLP contract has no functionality to handle these leftover WETHs which is lowering the efficiency of the reLP'ing process.

Proof of Concept

In UniswapV2 add liquidity is performed as follows: https://github.com/Uniswap/v2-periphery/blob/0335e8f7e1bd1e8d8329fd300aea2ef2f36dd19f/contracts/UniswapV2Router02.sol#L33-L60 https://github.com/Uniswap/v2-core/blob/ee547b17853e71ed4e0101ccfd52e70d5acded58/contracts/UniswapV2Pair.sol#L110-L131

So, if reserves of a hypothetical pool are 100tokenA and 50tokenB and if we need to add 10 tokenA then we exactly need 5 tokenB too. https://github.com/Uniswap/v2-periphery/blob/0335e8f7e1bd1e8d8329fd300aea2ef2f36dd19f/contracts/libraries/UniswapV2Library.sol#L36-L40

Now, let's assume the V2 pool has 1000rDPX, 100WETH, 6 * 1e18 totalSupply and rDPX reserve has 1000rDPX. https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L217-L237 we will calculate the lpToRemove as 0.06 and removing 0.06 LP tokens will give us: 1.13WETH 11.38rDPX

The UniV2 pool now has: 98.87WETH 988.62rDPX

Now we will swap half of the WETH to rDPX: 98.87 * 988.62 = k (98.87 + (1.13 / 2)) * (988.62 - dx) = k dx = 5.617 rDPX.

Now the contract holds 0.565WETH and 5.617rDPX which will be added liquidity back to UniV2 pool. UniV2 pool latest reservs were: 99.435WETH 983rDPX

When we follow the add liquidity steps in UniV2 the contract will add liquidity with the following amounts: full amount of rDPX, which is 5.617 rDPX and 0.563WETH. 0.002WETH will be idle in the contract. This can happen more often if the liquidity to be removen or reLPFactor being very high or very low. Idle WETH should be sweeped to Core contract in these cases.

Tools Used

Add a sweep functionality for WETH to handle leftover WETH's.

Assessed type

Uniswap

#0 - c4-pre-sort

2023-09-10T10:41:58Z

bytes032 marked the issue as duplicate of #1286

#1 - c4-pre-sort

2023-09-11T15:38:26Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-18T12:13:43Z

GalloDaSballo marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L372-L396 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L790-L808 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L462-L524

Vulnerability details

Impact

APP contract works with epochs and every epoch is default to 1 week. After every epoch the new funding rate will be calculated. If the core contract wants to pay the funding for a specific epoch to APP contract then it must needs to call provide funding in a precised time. However, if someone frontruns this tx or simply calls before the core contract it will be disaster for the core contract because it will not be able to pay the funding for a given epoch.

Proof of Concept

This is the function admin needs to call in Core contract to pay funding to the APP contract: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L790-L808 As we can see inside this function Core contract calls the APP contracts payFunding() function. Here the payFunding() function in the APP contract: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L372-L393 As we can see this line must satisfy in order to pay the funding for the given epoch from Core contract to APP contract: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L376-L381 The problem here is that the accounting mechanism for the "fundingPaymentsAccountedFor[latestFundingPaymentPointer]". latestFundingPaymentPointer will be incremented permissionlessly by calling the "updateFundingPaymentPointer()" in the APP contract. If the epoch is already over the payment pointer will be incremented. If this this happens, the new fundingPaymentsAccountedFor[latestFundingPaymentPointer] will be 0 hence, the validation logic will revert and Core contract will not be able to pay the funding as its intended.

Tools Used

Manual

Make sure that the funding has paid before rolling over to the next epoch.

Assessed type

Other

#0 - c4-pre-sort

2023-09-15T07:58:16Z

bytes032 marked the issue as duplicate of #1674

#1 - c4-judge

2023-10-20T19:40:57Z

GalloDaSballo changed the severity to QA (Quality Assurance)

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