search

sherlock-audit / 2024-03-arrakis-judging

2 stars 2 forks source link

Ocean_Sky - Updating maxSlippagePIPS can be DOSed and may cause losses to the vault. #47

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

Ocean_Sky

medium

Updating maxSlippagePIPS can be DOSed and may cause losses to the vault.

Summary

Updating maxSlippagePIPS can be DOSed and may cause losses to the public vault.

Vulnerability Detail

Per the contest's README page, public vault owner is "RESTRICTED" which indicates any attack or griefing by owner that can cause loss of funds or harm to users is valid in this contest.

image

The public vault owner has capability to update the following information (see function updateVaultInfo below) and one of these items is the maxSlippagePIPS (line 488). MaxSlippagePIPS is used as a security measure in rebalance function to ensure the difference between before and after vault balances is within the slippage tolerance set by the owner.

472:     function updateVaultInfo(SetupParams calldata params_)
473:         external
474:         whenNotPaused
475:         onlyWhitelistedVault(params_.vault)
476:         onlyVaultOwner(params_.vault)
477:     {
478:         _updateParamsChecks(params_);
479:         VaultInfo memory info = vaultInfo[params_.vault];
480: 
481:         vaultInfo[params_.vault] = VaultInfo({
482:             lastRebalance: info.lastRebalance,
483:             cooldownPeriod: params_.cooldownPeriod,
484:             oracle: params_.oracle,
485:             executor: params_.executor,
486:             maxDeviation: params_.maxDeviation,
487:             stratAnnouncer: params_.stratAnnouncer,
488:             maxSlippagePIPS: params_.maxSlippagePIPS,
489:             managerFeePIPS: info.managerFeePIPS
490:         });
491: 
492:         emit LogSetManagementParams(
493:             params_.vault,
494:             address(params_.oracle),
495:             params_.maxSlippagePIPS,
496:             params_.maxDeviation,
497:             params_.cooldownPeriod,
498:             params_.executor,
499:             params_.stratAnnouncer
500:         );
501:     }

The Issue

However, a compromised or malicious public vault owner can intentionally abandoned or delay the update of maxSlippagePIPS to whenever how long he wants it to be delayed. Only the public vault owner can access this updateVaultInfo function and the users or liquidity providers of the vault will be affected.

For example, the update of maxslippage could be from previous 5% to current 1%, this is already 4% slippage difference. So everytime, the rebalance will be executed, the vault will absorb the difference and the result may decrease the overall value of the vault, thus resulting losses to users.

Proof of Concept

Consider this scenario:

  1. There is a change want to be made by the public vault stakeholders and suggest to change the max slippage rate from 5% to 1%. The Arrakis DAO accepted the proposal change and ordered the public vault owner to change the rate as per proposal.

  2. Unfortunately, during this time, the public vault owner address has been compromised by malicious actor. The proposal change has been left in unknown status.

  3. At this point, two things can happen either

    a. The executor of the vault still don't know what's happening and still continued to execute rebalance with outdated maxslippage rate of 5% instead of 1%. In this case, the vault stakeholders will be left in disappointment as they witnessed the vault value shrink in unexpected rate.

    b. The executor of the vault know what's happening and decided to not execute the rebalance operation. However, this can be considered technically as denial of service since rebalance function can't be executed with proper max slippage rate approved by DAO.

Impacts

  1. Losses to the vault value if the rebalance is continued to be executed with outdated maxslippage rate.

  2. Rebalance may be paused as part of security measure to minimized the damage but this will technically mean loss of core functionality of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisStandardManager.sol#L488 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisStandardManager.sol#L411-L412

Tool used

Manual Review

Recommendation

Implement a backup system just in case the restricted role such as Public Vault Owner has been compromised or gone rogue.

cu5t0mPeo commented 2 months ago

The execution by the public vault owner is subject to a 2-day delay each time. During this period, users can consider the risk and reallocate their funds accordingly.