Dopex - wallstreetvilkas's results

Lines of code

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

Vulnerability details

Impact

The funding payment pointer is not updated at the beginning of the bond() function in RdpxV2Core.sol. Because of that, we can bond() when block.timestamp == nextFundingPaymentTimestamp (https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L563) which leads to incorrect calculations.

If block.timestamp is the same as the end time of the current epoch, malicious users can call the bond() function in the core contract and purchase options at expiry time of 0, and force the core contract to withdraw more rDPX from the rDPX reserve than needed.

Inside bond() we first calculate the bond cost: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L904-L907

The problem lies here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1192-L1194 Because the payment pointer is not updated at the start of the bond() function, the nextFundingPaymentTimestamp returns current epoch's end time. As mentioned above, if the user calls bond() when block.timestamp == nextFundingPaymentTimestamp(), this means that the time to expiry will be 0. Normally, the payment pointer should be updated, and at this point, the time to expiry would have to be the end of the new epoch, but because the payment pointer is not updated, we can purchase the option at the end of the current epoch.

There's another problem: once the bond cost is calculated, we then purchase the options: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L919-L922

And the premium calculated here is entirely different, because the time to expiry is now the next epoch's, because the payment pointer is updated in the purchase() function of the Atlantic Perpetual Vault: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L267

So,

1. The premium in the bond cost calculation is entirely different from the one that we calculate while purchasing options from the APP. Because of this, more rDPX is withdrawn from the rDPX reserves than needed because the discountReceivedInWeth calculated here is larger than supposed to be: Here's the discount calculation: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L670-L679 And here we can see that we subtract the premium from wethRequired: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L924

2. Users can purchase options at expiry time of 0, which shouldn't be possible.

Summary: If we bond() at the very end of the current epoch, it still allows us to purchase options, even though the expiry time is 0. The bond cost calculation uses the old epoch's end time, while the premium returned by the purchase() function in APP uses the updated epoch's end time, this leads to a difference in premiums calculated, so more rDPX is withdrawn from reserves.

Proof of Concept

Let's assume that the current payment pointer is 1.

1. Alice waits until block.timestamp == nextFundingPaymentTimestamp(), or basically the end of 1st epoch.

2. Alice calls the bond() function in RdpxV2Core.sol: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L894

3. The contract, then calculates Alice's bond cost (https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L904-L907), but uses the old payment pointer, 1, in the calculation. Because timeToExpiry is 0, the premium is 0, because it's the end of the epoch.

4. Then, the core contract purchases the options, but in purchase() (https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L267) payment pointer is updated to 2, and the new premium is calculated as if timeToExpiry was the end of the 2nd epoch.

5. So the premium here: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L921 is calculated as if it was the new epoch (2nd), but the bond cost calculation calculated it as if it was still the 1st epoch.

6. Because of this difference in premiums, the _transfer() https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L624 withdraws more rDPX from the reserves than required.

Tools Used

manual review

Recommended Mitigation Steps

Here's the modified code:

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

/* 
Here's the change, we update the funding payment pointer here, so that the bond cost 
is calculated correctly and users can't purchase options with 0 expiry time.
*/
updateFundingPaymentPointer();

// Compute the bond cost
(uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
  _amount,
  rdpxBondId
);

IERC20WithBurn(weth).safeTransferFrom(
  msg.sender,
  address(this),
  wethRequired
);

// update weth reserve
reserveAsset[reservesIndex["WETH"]].tokenBalance += wethRequired;

// purchase options
uint256 premium;
if (putOptionsRequired) {
  premium = _purchaseOptions(rdpxRequired);
}

_transfer(rdpxRequired, wethRequired - premium, _amount, rdpxBondId);

// Stake the ETH in the ReceiptToken contract
receiptTokenAmount = _stake(_to, _amount);

// reLP
if (isReLPActive) IReLP(addresses.reLPContract).reLP(_amount);

emit LogBond(rdpxRequired, wethRequired, receiptTokenAmount);

}

Assessed type

Context