BugHunter101 - A malicious `Timelock` executor can launch an in-front attack by calling the `sendPortfolio()` function when attacker discover the user calling `withdrawCollateral()` . This will cause user calling `withdrawCollateral()` revert and DoS.

[sherlock-admin]

BugHunter101

medium

A malicious Timelock executor can launch an in-front attack by calling the sendPortfolio() function when attacker discover the user calling withdrawCollateral() . This will cause user calling withdrawCollateral() revert and DoS.

Summary

A malicious Timelock executor can launch an in-front attack by calling the sendPortfolio() function when attacker discover the user calling withdrawCollateral() . This will cause user calling withdrawCollateral() revert and DoS.

Vulnerability Detail

As we can see, withdrawCollateral will check collateral - _getPortfolio(token) >= amount, if Alice withdraw amount=100 , and collateral = _getBalance(token) = 100 , and then, attacker calls sendPortfolio() setting receiver is Alice ,and transfer amount=1 using in-front attack. So now, collateral - _getPortfolio(token) = 100 - 1 = 99 ,but amount=100 ,so it will revert.

    function withdrawCollateral(address token, uint256 amount) external onlyGuardianOrWithdrawer nonReentrant {
        _checkAmountPositive(amount);

        uint256 collateral = _getBalance(token);
        _require(collateral - _getPortfolio(token) >= amount, Errors.POOL_BALANCE_INSUFFICIENT);//@audit

        _setBalance(token, collateral - amount);

        IERC20(token).safeTransfer(msg.sender, amount);

        emit CollateralWithdrawn(token, msg.sender, amount);
    }

sendPortfolio() call _sendPortfolio:

function sendPortfolio(address token, address receiver, uint256 amount)
        external
        onlyTimelock
        onlyPortfolio(receiver)
        nonReentrant
    {
        _sendPortfolio(token, receiver, amount);
    }

function _sendPortfolio(address token, address receiver, uint256 amount) internal virtual {
        AddressUtils.checkNotZero(token);
        AddressUtils.checkNotZero(receiver);
        _checkAmountPositive(amount);
        _require(receiver != address(this), Errors.RECEIVER_INVALID);

        uint256 portfolio = _getPortfolio(token);
        amount = amount.min(_getBalance(token) - portfolio);

        _require(amount > 0, Errors.POOL_BALANCE_INSUFFICIENT);

        _setPortfolio(token, portfolio + amount);

        IERC20(token).safeTransfer(receiver, amount);//@audit

        emit PortfolioSent(token, receiver, amount);
    }

Impact

This will cause user calling withdrawCollateral() revert and DoS.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/InsurancePool.sol#L102

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/InsurancePool.sol#L136

Tool used

Manual Review

Recommendation

check msg.sender == receiver in sendPortfolio()