sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

PRAISE - Wrong logic in _sendPortfolio() found in PoolBalances.sol will cause illicit gain of tokens #115

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

PRAISE

medium

Wrong logic in _sendPortfolio() found in PoolBalances.sol will cause illicit gain of tokens

Summary

There will be illicit gain of tokens when transferring tokens via _sendPortfolio().

Vulnerability Detail

The problem here is that the amount being transferred out is added to the prev portfolio balance instead of it being deducted.

 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);//@audit-info `amount` should be deducted here since the `amount` is sent out

        IERC20(token).safeTransfer(receiver, amount);

        emit PortfolioSent(token, receiver, amount);
    }

so whenever the _sendPortfolio() function is used to send tokens, the amount sent out will be added and used to increment the porfolio balance mapping.

 mapping(address => uint256) internal _portfolio;

Impact

Tokens will be gained whenever tokens are transferred via sendPortfolio() function.

Code Snippet

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

Tool used

Manual Review

Recommendation

deduct the amount from the prev portfolio balance since amount is being transferred out