cu5t0mPe0 - Did not check whether the router is legal

[sherlock-admin2]

cu5t0mPe0

high

Did not check whether the router is legal

Summary

During swap action, the module will use a call instruction to invoke the target address to complete the swap, but it does not check if the router is compliant.

Vulnerability Detail

ValantisHOTModule.swap does not check if the router involved in the swap is whitelisted. The executor can exploit this by setting the router to the HOT contract and calling setAMMFees, or by setting the router to the SovereignPool contract and calling functions that are restricted to PoolManager only.

The impact is significant; users can even attempt to change the PoolManager to themselves and then call claimPoolManagerFees to steal funds.

Impact

The executor destroys the pool or steals the amount at will

Code Snippet

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

Tool used

Manual Review

Recommendation

We should use a whitelist mechanism to restrict the addresses that can be used for swaps.

[cu5t0mPeo]

Escalate The router can be set to any value, and attackers can manipulate payloads freely. Many other Watsons have also noticed this, but it seems they have been rejected without any justification.

[sherlock-admin3]

Escalate The router can be set to any value, and attackers can manipulate payloads freely. Many other Watsons have also noticed this, but it seems they have been rejected without any justification.

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]

No correct specific attack path shown.

1. Calling HOT.setAMMFees will revert due to expectedMinReturn_ not being met
2. Calling setPoolManager to anything other than address(0) will revert due to expectedMinReturn_ not being met

[WangSecurity]

And adding to the comment above, the issues related to exploiting the protocol with custom bytes data require a POC:

PoC is required for all issues falling into any of the following groups: non-obvious ones with complex vulnerabilities/attack paths issues for which there are non-trivial limitations/constraints on inputs, to show that the attack is possible despite those

Hence, planning to reject the escalation and leave the issue as it is.

[WangSecurity]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• cu5t0mPeo: rejected