sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

Shubham - Funds can be burned & withdrawn earlier due to incorrect `unboundTime` #229

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Shubham

medium

Funds can be burned & withdrawn earlier due to incorrect unboundTime

Summary

The withdrawal process cannot start until the unboundtime for a particular user has passed. But incorrect calculations show that the withdraw can be initiated before the desired period has passed.

Vulnerability Detail

The unbondShares function is used to set the unboundTime for a particular user. The unbondPeriod has been set to 2 days & unbondRoundOff to 1 day. Thus when calculating unboundTime, it yields an answer that is between 1-2 days.

function unbondShares(uint shares) external {
        address usr = _msgSender();
        require(shares <= balanceOf(usr), "unbonding_too_much");
        uint _now = _blockTimestamp();
        uint unbondTime = ((_now + unbondPeriod) / unbondRoundOff) * unbondRoundOff;    --------------> @audit
        unbond[usr] = UnbondInfo(shares, unbondTime);
        emit Unbonded(usr, shares, unbondTime, _now);
    }

Say the current block.timestamp is 1688389590 Calculating unboundTime results in 1688515200 Taking the difference results 1,25,610 which is roughly 1.4 days.

Thus bypassing the following require statements.

 function _withdrawFor(address user, uint shares, address to) internal returns (uint amount) {
        // Checks
        require(unbond[user].shares >= shares, "withdrawing_more_than_unbond");
        uint _now = _blockTimestamp();
        require(_now >= unbond[user].unbondTime, "still_unbonding");                         ------------->bypass
        require(!_hasWithdrawPeriodElapsed(_now, unbond[user].unbondTime), "withdraw_period_over"); ------>bypass
        .........................................................

Impact

Calling the withdraw or withdrawFor function can lead to early withdrawal bypassing the unbounding time, thus breaking the protocol's guideline. This leads to burning shares & transfer of tokens earlier than expected which might lead to loss of funds to the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/InsuranceFund.sol#L116-L123 https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/InsuranceFund.sol#L205-L210

Tool used

Manual Review

Recommendation

Simply add unbondPeriod to the current timestamp rather than dividing & multiplying with unbondRoundOff.