code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

Removed tokens can't be withdrawn from vault #69

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

hickuphh3

Vulnerability details

Impact

A token that has been added and subsequently removed through the manager will cause users' deposited funds in that token to be trapped in the vault.

Recommended Mitigation Steps

The vault's deposit and withdraw modifier should be separate, such that users are still able to withdraw previously added (but subsequently removed) tokens from the vault.

uN2RVw5q commented 2 years ago

The vault's deposit and withdraw modifier should be separate, such that users are still able to withdraw previously added (but subsequently removed) tokens from the vault.

Allowing this doesn't feel right to me.

As long as there is some token available, the user will be able to withdraw their share in an equivalent token, even if that token is different from the deposited token.

I would consider this to be a non-issue. Perhaps this can be converted into a documentation tag. Add some documentation about checking before a token is removed in Manager contract and see if an equivalent token is present in the vault sounds reasonable. In the worst case, a removed token can still be added back by the manager, and therefore, the tokens should be safe.

GalloDaSballo commented 2 years ago

Agree with the original warden finding reduced severity to Low

The admin strategist has the ability to remove tokens, this removal can impact accounting, notably Vault.sol's balanceOfThis

May want to add a check on removeToken