sherlock-audit / 2023-03-Y2K-judging

7 stars 1 forks source link

ak1 - `VaultV2.sol` : `withdraw` is not providing fair logic to end user. #496

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

medium

VaultV2.sol : withdraw is not providing fair logic to end user.

Summary

User can call the withdraw function to widthdraw.

This function does not have any min amount to witdraw. like how the carosul.sol has for deposit and depositETH.

when we look at the function logic, it first burns the tokens and then transfers the shares only if if (entitledShares > 0) User can call any value to withdraw. given these type scenario, for a given small amount value the , the previewWithdraw could return zero value.

In the above scenario, user will not get anything instead the assets are burned.

Vulnerability Detail

           _burn(_owner, _id, _assets);

    uint256 entitledShares;

    if (epochNull[_id] == false) {
        entitledShares = previewWithdraw(_id, _assets);
    } else {
        entitledShares = _assets;
    }
    if (entitledShares > 0) {
        SemiFungibleVault.asset.safeTransfer(_receiver, entitledShares);
    }

in the above code snip from witdraw function, it burns assets and transfers the entitledShares only if entitledShares > 0

Impact

loss of asset to end user.

user may not aware of this and try to call the function with minimum asset value for their need.

user may not be wanted to withdraw large amount of value always.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L169-L180

Tool used

Manual Review

Recommendation

update the logic as shown below,

    _burn(_owner, _id, _assets); 

    uint256 entitledShares;

    if (epochNull[_id] == false) {
        entitledShares = previewWithdraw(_id, _assets);
        require( entitledShares !=0 ); ----------------> add this line

    } else {
        entitledShares = _assets;
    }
    if (entitledShares > 0) {
        SemiFungibleVault.asset.safeTransfer(_receiver, entitledShares);
    }