code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

PerpetualAtlanticVault::setAddress() is prone to frontrunning #1116

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L181

Vulnerability details

Issue

The PerpetualAtlanticVault::setAddress() function is prone to frontrunning. Below is the core update that the PerpetualAtlanticVault::setAddress() function performs.

...
addresses = Addresses({
  optionPricing: _optionPricing,
  assetPriceOracle: _assetPriceOracle,
  volatilityOracle: _volatilityOracle,
  feeDistributor: _feeDistributor,
  rdpx: _rdpx,
  perpetualAtlanticVaultLP: _perpetualAtlanticVaultLP,
  rdpxV2Core: _rdpxV2Core
});
...

Link: Code snippet

This updates various addresses which are then used in other functions throughout the contract. If an admin updates the above addresses using the PerpetualAtlanticVault::setAddress() function, this can be frontrun and can lead to the usage of the old addresses.

Impact

The most critical of all the above addresses is the perpetualAtlanticVaultLP address. Throughout the contract there are several uses of this address, including transfer of different tokens. If the previous perpetualAtlanticVaultLP contract address is compromised, then the transaction to update the perpetualAtlanticVaultLP can be frontrun and transactions will be performed on the old compromised perpetualAtlanticVaultLP contract address.

Tools Used

Manual analysis

Recommended Mitigation Steps

If any addresses need to be updated in the contract, then the pause function should be utilised so any frontrunning attempts can be avoided.

Assessed type

Other

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

bytes032 commented 1 year ago

There's no reason to front run here

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid