sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

dipp - Anyone may swap any user's quote tokens for base tokens if that user has approved ```PerpDepository.sol``` to spend quote tokens #382

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

dipp

high

Anyone may swap any user's quote tokens for base tokens if that user has approved PerpDepository.sol to spend quote tokens

Summary

A malicious user could call the rebalanceLite function in PerpDepository.sol and specify any account address that has given approval to the depository. This may result in the account losing their quote tokens in exchange for base tokens.

Vulnerability Detail

The rebalanceLite function allows any account to be specified that has approved the PerpDepository.sol contract.

    function rebalanceLite(
        uint256 amount,
        int8 polarity,
        uint160 sqrtPriceLimitX96,
        address account
    ) external nonReentrant returns (uint256, uint256) {
        if (polarity == -1) {
            return
                _rebalanceNegativePnlLite(amount, sqrtPriceLimitX96, account);
        } else if (polarity == 1) {
            // disable rebalancing positive PnL
            revert PositivePnlRebalanceDisabled(msg.sender);
            // return _rebalancePositivePnlLite(amount, sqrtPriceLimitX96, account);
        } else {
            revert InvalidRebalance(polarity);
        }
    }

Since only negative PnL rebalances are allowed, the _rebalanceNegativePnlLite function is called where the account must provide all quote tokens for the rebalance and receives the base tokens returned in the swap.

    function _rebalanceNegativePnlLite(
        uint256 amount,
        uint160 sqrtPriceLimitX96,
        address account
    ) private returns (uint256, uint256) {
        uint256 normalizedAmount = amount.fromDecimalToDecimal(
            ERC20(quoteToken).decimals(),
            18
        );

        _checkNegativePnl(normalizedAmount);
        IERC20(quoteToken).transferFrom(account, address(this), amount);
        IERC20(quoteToken).approve(address(vault), amount);
        vault.deposit(quoteToken, amount);

        bool isShort = false;
        bool amountIsInput = true;
        (uint256 baseAmount, uint256 quoteAmount) = _placePerpOrder(
            normalizedAmount,
            isShort,
            amountIsInput,
            sqrtPriceLimitX96
        );
        vault.withdraw(assetToken, baseAmount);
        IERC20(assetToken).transfer(account, baseAmount);

        emit Rebalanced(baseAmount, quoteAmount, 0);

        return (baseAmount, quoteAmount);
    }

Impact

The account used in rebalanceLite could have there quoteTokens exchanged for baseTokens. If this occurs during a time of high volatility, the account may suffer a loss.

Code Snippet

PerpDepository.sol:rebalanceLite#L597-L602

PerpDepository.sol:_rebalanceNegativePnlLite#L615-L644

Tool used

Manual Review

Recommendation

Consider using msg.sender instead of account in the rebalanceLite function.

Duplicate of #288