sherlock-audit / 2024-03-arrakis-judging

2 stars 2 forks source link

cu5t0mPe0 - The executor can prevent the manager from receiving the manager fee. #18

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

cu5t0mPe0

medium

The executor can prevent the manager from receiving the manager fee.

Summary

When the vault creates a new module, its _managerFeePIPS value will be 0. The executor can exploit this to prevent the manager from receiving the fee.

Vulnerability Detail

The executor can call ArrakisStandardManager.setModule to set a new module. When the manager calls setManagerFeePIPS to set the managerFeePIPS of a private vault, the private vault owner can create a new module and then use the executor to call setModule. As a result, the managerFeePIPS of the private vault will become 0. These actions will not affect ArrakisStandardManager.vaultInfo, so the manager will still believe that the managerFeePIPS of the private vault has not changed. Ultimately, when the manager calls withdrawManagerBalance to withdraw the manager fee, the amount available to withdraw will be 0.

Consider the following scenario:

  1. Alice is the owner and executor of a private vault. The manager wants to collect the manager fee from Alice's private vault, so they set the managerFeePIPS to 1000.
  2. Alice realizes she will be charged a manager fee. After the manager calls finalizeIncreaseManagerFeePIPS, Alice immediately calls whitelistModules and setModule to set a new module. At this point, the new module's _managerFeePIPS value will be 0.
  3. Alice then calls setALMAndManagerFees (this function internally calls pool.setPoolManagerFeeBips(_managerFeePIPS / 1e2)), ensuring that the poolManagerFeeBips value in the pool remains 0.
  4. When the manager queries vaultInfo to check Alice's private vault, they see that the managerFeePIPS is still 1000 and believe the setting was successful. Ultimately, this prevents the manager from receiving the intended manager fee.

Impact

Manager cannot receive manager fee

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/ArrakisStandardManager.sol#L429-L441

Tool used

Manual Review

Recommendation

It is recommended to call IArrakisLPModule(IArrakisMetaVault(vault_).module()).setManagerFeePIPS(vaultInfo[vault_].managerFeePIPS); in the ArrakisStandardManager.setModule function.

cu5t0mPeo commented 2 months ago

Escalate This must be a valid issue:

  1. The question was rejected without any reason.
  2. When creating a new module, the value of _managerFeePIPS will be 0. Subsequently, when calling the setALMAndManagerFees function to set the Manager fee, the Manager cannot detect that the user's Manager fee is abnormal, as the Manager fee in vaultInfo within ArrakisPublicVaultRouter will not change.
sherlock-admin3 commented 2 months ago

Escalate This must be a valid issue:

  1. The question was rejected without any reason.
  2. When creating a new module, the value of _managerFeePIPS will be 0. Subsequently, when calling the setALMAndManagerFees function to set the Manager fee, the Manager cannot detect that the user's Manager fee is abnormal, as the Manager fee in vaultInfo within ArrakisPublicVaultRouter will not change.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

0xjuaan commented 2 months ago

invalid because the private vault owner and executor are trusted

cu5t0mPeo commented 2 months ago

@Gevarist Anyone can create a private vault, so I believe it shouldn't be trusted by the Manager. Sponsor needs to confirm some key points:

WangSecurity commented 2 months ago

As I understand based on the README, the owner/admin of the private vault is indeed trusted.

Anyone can create a private vault, so I believe it shouldn't be trusted by the Manager. Sponsor needs to confirm some key points: The owner of the private vault should be trusted by the current private vault, not by the Manager. The private vault owner will not perform any malicious actions on the private vault, but it cannot be ensured that they won't perform malicious actions on the Manager.

Is it based on docs/info or a logical assumption?

cu5t0mPeo commented 2 months ago

@WangSecurity The documentation does indeed state that the owner/admin is trusted. However, since anyone can call the private vault, I believe the sponsor may not be aware of the attack vector between private vaults. Therefore, I think this issue needs to be confirmed again by the sponsor.

Another point is that this attack is also effective against the public vault. I did not include it in the report because the sponsor mentioned on Discord that the public vault is controlled by a time lock with a 2-day buffer period, allowing users and managers to detect risks. At that time, I was unsure if the manager would detect this issue. Hence, I did not mention the public vault in that context. However, in reality, the manager would also be unable to detect if _managerFeePIPS is set to 0 because the manager would refer to ArrakisStandardManager.vaultInfo for information, but ArrakisStandardManager.vaultInfo is actually incorrect.

cu5t0mPeo commented 2 months ago

Is it based on docs/info or a logical assumption?

logical assumption The owner of a private vault should be trusted for their own vault, but they may not be trusted by the manager, as private vaults can be created by anyone and the manager can also charge fees to any vault.

WangSecurity commented 2 months ago

Still, based on README, we have to assume they're trusted, regardless of sponsor confirming the issue (remember, this doesn't effect the final decision). Moreover, setModule can only be called only by whitelisted vaults which are added by the manager (as I understand). Hence, we should assume these are trusted.

Planning to reject the escalation and leave the issue as it is.

WangSecurity commented 2 months ago

Result: Invalid Unique

sherlock-admin4 commented 2 months ago

Escalations have been resolved successfully!

Escalation status: