sherlock-audit / 2024-03-arrakis-judging

2 stars 2 forks source link

cu5t0mPe0 - The executor takes away all the funds during `setModule` #15

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

cu5t0mPe0

high

The executor takes away all the funds during setModule

Summary

During setModule, the executor will call initializePosition to deposit funds into the pool. Subsequently, the executor can call withdraw to withdraw all the funds.

Vulnerability Detail

When the executor sets a new module using Vault, they use a call to invoke any function of the new module.

According to the sponsor's description, the executor is not trusted, but it is safe for setModule and its related implementations, but in fact this is not the case.

This is because the executor can call the withdraw function of the new module through Vault's setModule function. Since the call originates from the Vault, msg.sender is set to the Vault, bypassing permission restrictions.

Consider the following scenario:

  1. A malicious executor calls initializePosition to redeposit funds into the pool.
  2. Due to the for loop, the malicious executor then calls ValantisHOTModulePublic.withdraw to withdraw all funds to a target address. This occurs because withdraw is called by the vault, thereby bypassing permission checks.

Impact

executors can steal all funds

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/abstracts/ArrakisMetaVault.sol#L130-L130

Tool used

Manual Review

Recommendation

We should use a whitelist mechanism to restrict the functions that the executor can call.

Duplicate of #50

0xjuaan commented 2 months ago

Escalate

Valid dupe of #50

sherlock-admin3 commented 2 months ago

Escalate

Valid dupe of #50

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.

WangSecurity commented 2 months ago

Agree with the escalation, planning to accept it and duplicate with #50.

WangSecurity commented 2 months ago

Result: High Duplicate of #50

sherlock-admin4 commented 2 months ago

Escalations have been resolved successfully!

Escalation status: