sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

Nyx - Users might lose funds when using withdrawAllowance(). #368

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Nyx

high

Users might lose funds when using withdrawAllowance().

Summary

Users always lose some funds when user.withdrawalAllowance is greater than vault balance.

Vulnerability Detail

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L238-L245 When using withdrawAllowance(), it checks the vault balance and if the vault balance is lower than user.withdrawalAllowance, value will be equal vault balance.

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L242 It reverts when the difference is higher than maxDivergenceWithdraws, but still, users are losing funds.

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L175 It deletes user.withdrawalAllowance instead of user.withdrawalAllowance - value.

Impact

Users will lose their remaining funds and can't receive them next time.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L166-L179

Tool used

Manual Review

Recommendation

user.withdrawalAllowance = user.withdrawalAllowance - value;

So When the vault has funds, users can withdraw their remaining funds.

Duplicate of #345