search

hats-finance / Tokemak-0x4a2d708ea6b0c04186ecb774cfad1e50fb5efc0b

0 stars 0 forks source link

`ensureDestinationRegistered` does not reverts if a destination is queued for removal #20

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: @BenRai1 Twitter username: @BenBurgerLG Submission hash (on-chain): 0x7bd6e39cfd3140bf0e8eed0c87ac21b602d715c4a39bcf4d20e652c56bb99c19 Severity: high

Description: Description\ The function LMPStrategy:verifyRebalance is called to "verify that a rebalance (swap between destinations) meets all the strategy constraints". Within this function the function validateRebalanceParams() is called to verifiy if the provided parameters meet the requirements of the strategy. One of the check during this validation of the parameters is done by calling ensureDestinationRegistered. This fuction is supposed to check if the provided destinations are registered in the vault meaning if they are allowed to be used for the rebalanceing. The function should revert if the destination is not registered or if it is quewed for removal. The issue is that this function does not revert if the destination is not registered but queued for removal. A destination is normaly queued for removal becasue it is not good to invest in it any more, e.g. because the returns are lower than other alternatives or even worse, because it was hacked. Investing in this alternative could lead to substantial lose of funds for the vault.

Attack Scenario\ Rebalancing is done with a destination which is quewed for removal which can lead to substantial lose of funds to the vault

Attachments

  1. Proof of Concept (PoC) File Will be added later in the comment in github

  2. Revised Code File (Optional) Remove the additional braket when checking if the destination is not registered or quewed for removal Change this: if (!(lmpVault.isDestinationRegistered(dest) || lmpVault.isDestinationQueuedForRemoval(dest)))

to this:

if (!lmpVault.isDestinationRegistered(dest) || lmpVault.isDestinationQueuedForRemoval(dest))

codenutt commented 5 months ago

It is valid for a rebalance to include a destination that has been queued for removal. We have to first exit the destination fully before fully removing and rebalancing out of that destination is one way for that to occur.