sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

yongkiws - `Withdraw` and `Cancel` time can be circumvented _recipientBalance() #44

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

yongkiws

medium

Withdraw and Cancel time can be circumvented _recipientBalance()

Summary

The balance is calculated using the startTime() of the stream, the amount of tokens that should be streamed, and the current rate at which tokens are being streamed. If the current block's timestamp is before the start time of the stream, the balance is 0. If the current block's timestamp is after the stop time of the stream, the balance is the total amount of tokens that should be streamed. Otherwise, the balance is calculated by multiplying the elapsed time by the rate per second, and dividing by a rate multiplier. Finally, the function takes any withdrawals into account by subtracting them from the balance.

Vulnerability Detail

someone to call a function that changes the time period elapsedTime() and _recipientBalance() , an attacker can call the function to reduce the time period and then immediately withdraw funds that should be locked.

Impact

Summary

Code Snippet

-if the current block time is less than the start time. If so, it returns 0. -if the current block time is greater than the stop time. If so, it returns the token amount. -Otherwise, it calculates the elapsed time between the start time and the current block time. -It then multiplies the elapsed time by the rate per second. -Finally, it divides the result by the rate per second to get the token balance.

 function _recipientBalance() internal view returns (uint256) {
        uint256 startTime_ = startTime();
        uint256 blockTime = block.timestamp;

        if (blockTime <= startTime_) return 0;

        uint256 tokenAmount_ = tokenAmount();
        uint256 balance;
        if (blockTime >= stopTime()) {
            balance = tokenAmount_;
        } else {
            // This is safe because: blockTime > startTime_ (checked above).
            unchecked {
                uint256 elapsedTime_ = blockTime - startTime_;
                balance = (elapsedTime_ * ratePerSecond()) / RATE_DECIMALS_MULTIPLIER;
            }
        }

        uint256 remainingBalance_ = remainingBalance;

        if (remainingBalance_ == 0) return 0;

        // Take withdrawals into account
        if (tokenAmount_ > remainingBalance_) {

            unchecked {
                uint256 withdrawalAmount = tokenAmount_ - remainingBalance_;
                balance -= withdrawalAmount;
            }
        }

        return balance;
    }

Tool used

Manual Review

Recommendation

The unlock timestamp should be increased by duration each time, instead of being reset to the duration.

davidbrai commented 1 year ago

I don't understand the described issue.

hrishibhat commented 1 year ago

Agree with Sponsor. The issue does not clearly describe the attack.