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
Rank: 35/189
Findings: 10
Award: $571.11
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: klau5
Also found by: 0x3b, 0xCiphky, 0xDING99YA, 0xWaitress, 0xbranded, 0xc0ffEE, 0xklh, 0xsurena, 0xvj, ABA, AkshaySrivastav, Anirruth, Aymen0909, Baki, Blockian, BugzyVonBuggernaut, DanielArmstrong, Evo, GangsOfBrahmin, HChang26, Inspex, Jiamin, Juntao, Kow, Krace, KrisApostolov, LFGSecurity, LokiThe5th, Mike_Bello90, Norah, Nyx, QiuhaoLi, RED-LOTUS-REACH, SBSecurity, Snow24, SpicyMeatball, T1MOH, Tendency, Toshii, Udsen, Yanchuan, __141345__, ak1, asui, auditsea, ayden, bart1e, bin2chen, blutorque, carrotsmuggler, chaduke, chainsnake, circlelooper, clash, codegpt, crunch, degensec, dirk_y, ge6a, gjaldon, grearlake, jasonxiale, juancito, ke1caM, kodyvim, kutugu, ladboy233, lanrebayode77, mahdikarimi, max10afternoon, mert_eren, nirlin, nobody2018, oakcobalt, parsely, peakbolt, pks_, pontifex, ravikiranweb3, rokinot, rvierdiiev, said, savi0ur, sces60107, sh1v, sl1, spidy730, tapir, tnquanghuy0512, ubermensch, visualbits, volodya, wintermute
0.0098 USDC - $0.01
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
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.
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
Manual
Do not rely on balanceOf function when accounting the collateral balance in the vault LP contract.
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
🌟 Selected for report: bart1e
Also found by: 0x3b, 0xCiphky, Aymen0909, HHK, Inspex, bin2chen, chaduke, gkrastenov, jasonxiale, josephdara, kodyvim, peakbolt, pep7siup, rokinot, rvierdiiev, tapir
181.367 USDC - $181.37
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.
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.
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.
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
🌟 Selected for report: said
Also found by: 0Kage, 0xCiphky, 0xkazim, 836541, AkshaySrivastav, Evo, HChang26, HHK, KrisApostolov, Neon2835, QiuhaoLi, Tendency, Toshii, bart1e, bin2chen, carrotsmuggler, chaduke, etherhood, gjaldon, glcanvas, josephdara, lanrebayode77, mahdikarimi, max10afternoon, nobody2018, peakbolt, qpzm, rvierdiiev, sces60107, tapir, ubermensch, volodya
17.313 USDC - $17.31
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
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.
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.
Manual
First accrue the fundings and then call the previewDeposit to get the latest collateral balance in the contract.
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
🌟 Selected for report: 0xrafaelnicolau
Also found by: 0x111, 0xCiphky, 0xMosh, 0xWaitress, 0xc0ffEE, 0xkazim, 0xnev, 0xvj, ABAIKUNANBAEV, Aymen0909, Baki, ElCid, HChang26, HHK, Inspex, Jorgect, Kow, Krace, KrisApostolov, LFGSecurity, MiniGlome, Nyx, QiuhaoLi, RED-LOTUS-REACH, Talfao, Toshii, Vagner, Viktor_Cortess, Yanchuan, _eperezok, asui, atrixs6, bart1e, bin2chen, carrotsmuggler, chaduke, chainsnake, deadrxsezzz, degensec, dethera, dimulski, dirk_y, ether_sky, gizzy, glcanvas, grearlake, gumgumzum, halden, hals, kodyvim, koo, ladboy233, lanrebayode77, max10afternoon, minhtrng, mussucal, nobody2018, peakbolt, pontifex, qbs, ravikiranweb3, rvierdiiev, said, tapir, ubermensch, volodya, wintermute, yashar, zaevlad, zzebra83
0.0734 USDC - $0.07
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
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.
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
Manual
Decrement "totalWethDelegated" variable when users withdraws delegated WETH.
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
🌟 Selected for report: bin2chen
Also found by: 0Kage, 0xDING99YA, QiuhaoLi, Toshii, Yanchuan, carrotsmuggler, deadrxsezzz, ether_sky, flacko, gjaldon, kutugu, mert_eren, pep7siup, qpzm, said, sces60107, tapir, ubermensch
39.433 USDC - $39.43
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
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.
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.
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.
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
🌟 Selected for report: 0xWagmi
Also found by: 836541, Bauchibred, GangsOfBrahmin, Hama, IceBear, Inspecktor, Matin, MohammedRizwan, catellatech, erebus, lsaudit, niki, okolicodes, ravikiranweb3, tapir, vangrim, zaevlad
46.2486 USDC - $46.25
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
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.
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.
Manual
Introduce an minimum initial deposit cap so that vault ensures the division error will not occurs.
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
238.7514 USDC - $238.75
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
Detailed description of the impact of this finding.
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.
If the assets are 0, check the rDPX amount. If that's also 0 then revert. If not just give the rDPX to user.
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
🌟 Selected for report: Vagner
Also found by: 0Kage, 0xCiphky, 0xnev, ABAIKUNANBAEV, Aymen0909, Evo, KmanOfficial, MohammedRizwan, T1MOH, Viktor_Cortess, Yanchuan, ak1, alexzoid, bin2chen, codegpt, hals, ladboy233, mrudenko, nemveer, oakcobalt, peakbolt, pep7siup, qbs, said, savi0ur, tapir, wintermute, zaevlad, zzebra83
3.9186 USDC - $3.92
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.
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.
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.
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.
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)
🌟 Selected for report: degensec
Also found by: 0x3b, 0xnev, HChang26, KmanOfficial, QiuhaoLi, T1MOH, WoolCentaur, Yanchuan, ayden, bart1e, jasonxiale, kutugu, mert_eren, nirlin, peakbolt, peanuts, pep7siup, qpzm, tapir, ubermensch, wintermute
24.8267 USDC - $24.83
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.
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.
Add a sweep functionality for WETH to handle leftover WETH's.
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
🌟 Selected for report: juancito
Also found by: 0xDING99YA, 0xTiwa, 0xkazim, 0xnev, ABA, ArmedGoose, Aymen0909, Bauchibred, Evo, IceBear, KrisApostolov, MohammedRizwan, Nikki, QiuhaoLi, T1MOH, Toshii, WoolCentaur, Yanchuan, __141345__, asui, bart1e, carrotsmuggler, catellatech, chaduke, codegpt, deadrxsezzz, degensec, dethera, dirk_y, erebus, ether_sky, gjaldon, glcanvas, jasonxiale, josephdara, klau5, kodyvim, ladboy233, lsaudit, minhquanym, parsely, peakbolt, pep7siup, rvierdiiev, said, savi0ur, sces60107, tapir, ubermensch, volodya, zzebra83
19.1724 USDC - $19.17
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
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.
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.
Manual
Make sure that the funding has paid before rolling over to the next epoch.
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)