sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

PRAISE - The doCutVaultWithdrawFee() function will only cut fee if the blocktimestamp is less than withdrawVaultFeeWindowStartTime() + withdrawVaultFee Window #107

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

PRAISE

medium

The doCutVaultWithdrawFee() function will only cut fee if the blocktimestamp is less than withdrawVaultFeeWindowStartTime() + withdrawVaultFee Window

Summary

The doCutVaultWithdrawFee() function will only cut fee if the blocktimestamp is less than withdrawVaultFeeWindowStartTime() + withdrawVaultFee Window

Vulnerability Detail

According to this IF statement in the doCutVaultWithdrawFee() function

        if (
            block.timestamp <
            config.withdrawVaultFeeWindowStartTime() +
                config.withdrawVaultFeeWindow()
        ) {}

The _doCutFee function will only be called when doCutVaultWithdrawFee() function is called within a blocktimestamp < the given withdrawVaultFeeWindowStartTime() + withdrawVaultFeeWindow() time/blocktimestamp.

This is bad because i believe according to this comments by the devs

    /// @notice Cut vault withdraw fee when perform withdraw from Blueberry Money Market within the given window

and

        // Cut withdraw fee if it is in withdrawVaultFee Window

The _doCutFee() function is meant to be called when withdraw is performed from Blueberry Money Market within the given window i.e withdrawVaultFeeWindowStartTime() + withdrawVaultFeeWindow() time/blocktimestamp, but here cut withdraw fees will only happen if its called in a period less than the given withdrawVaultFee Window.

Impact

When withdraws are made from Blueberry Money Market within the given withdrawVaultFee Window, cut withdraw fee won't be done and treasury won't receive the withdraw fees.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/FeeManager.sol#L72-L89

Tool used

Manual Review

Recommendation

change the IF statement to this

        if (
            block.timestamp ==
            config.withdrawVaultFeeWindowStartTime() +
                config.withdrawVaultFeeWindow()
        ) {
sherlock-admin commented 1 year ago

escalate for 10 USDC

i believe this function in the feeManager won't work as intended because it expects the function to be called in a period/time earlier than the given withdraw vault fee window.

     function doCutVaultWithdrawFee(
        address token,
        uint256 amount
    ) external returns (uint256) {
        // Cut withdraw fee if it is in withdrawVaultFee Window
        if (
            block.timestamp < // the if statement expects the blocktimestamp to be < than the given withdraw window in the config
            config.withdrawVaultFeeWindowStartTime() +
                config.withdrawVaultFeeWindow()
        ) {
            return _doCutFee(token, amount, config.withdrawVaultFee());
        } else {
            return amount;
        }
    }

the impact here as stated by the watson is that When withdraws are made in the vaults within the given withdraw Vault Fee Window, the vault withdraw fees won't be deducted and treasury won't receive the withdraw fees.

Please Note: that HardVault's withdraw() function calls the doCutVaultWithdrawFee() function with the intention of it getting the withdraw fee from the vault to send it to the treasury here --https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/vault/HardVault.sol#L140

You've deleted an escalation for this issue.