branch_indigo - Malicious Product Owner Can Front Run Open Make/Take Position with High Maker/Taker Fees

[sherlock-admin]

branch_indigo

medium

Malicious Product Owner Can Front Run Open Make/Take Position with High Maker/Taker Fees

Summary

The product creation process is intended to be permissionless according to protocol docs and product owners are not trusted. A malicious product owner can front-run user openTake and openMake transactions with unrealistic maker/taker fees.

Vulnerability Detail

A product owner can change maker/taker fees through updateMakerFee and updateTakerFee in UParamProvider.sol. When there are no pending positions (pre-positions), the updated fee is directly written to _makerFeeand _takerFee storage. When there are pending positions, the updated fee will be written to _pendingFeeUpdates to be settled at the following oracle version. The only constraint is the fee cannot be more than 100% of the user position.

//UParamProvider.sol
    function updateMakerFee(UFixed18 newMakerFee) external onlyProductOwner settleProduct {
        if (!_noPendingPositions()) {
            _updatePendingMakerFee(newMakerFee);
        } else {
            _updateMakerFee(newMakerFee);
        }
    }

//UParamProvider.sol
    function _updateMakerFee(UFixed18 newMakerFee) private {
|>        if (newMakerFee.gt(UFixed18Lib.ONE)) revert ParamProviderInvalidParamValue();
        _makerFee.store(newMakerFee);
        emit MakerFeeUpdated(newMakerFee, _productVersion());
    }

There are two vulnerabilities. (1) the maker or taker fee can be updated to storage instantly. (2) The cap for maker fee and taker fee is insufficient to keep maker/taker fees at a reasonable level.

When there are no pending positions, a malicious product owner can spot a profitable large openMake or openTake transaction in the mempool, and front-run it by setting maker/taker fee at an unrealistic value of up to 100%. When the user open position transaction settles after fee update, the new fee percentage is multiplied by user position amount and instantly deducted from user collateral balance. As long as, the product owner choose a high percentage that target an over-collateralized user, the user transaction will go through passing the maintenance margin with immediate loss of collateral.

//Product.sol-openTakerFor()
        ...
        UFixed18 positionFee = amount.mul(latestOracleVersion.price.abs()).mul(takerFee());
        if (!positionFee.isZero()) {
 |>           controller().collateral().settleAccount(account, Fixed18Lib.from(-1, positionFee));
            emit PositionFeeCharged(account, latestOracleVersion.version, positionFee);
        }
        emit PositionFeeCharged(account, latestOracleVersion.version, positionFee);
        emit TakeOpened(account, latestOracleVersion.version, amount);

//OptimisticLedger.sol
    function settleAccount(
        OptimisticLedger storage self,
        address account,
        Fixed18 amount
    ) internal returns (UFixed18 newShortfall) {
        Fixed18 newBalance = Fixed18Lib.from(self.balances[account]).add(amount);
        if (newBalance.sign() == -1) {
            newShortfall = newBalance.abs();
            newBalance = Fixed18Lib.ZERO;
        }
        self.balances[account] = newBalance.abs();
        self.shortfall = self.shortfall.add(newShortfall);
    }

After user pre-position is created and user collaterals are lost, the product owner can then set maker or taker fee back to a reasonable level which writes to pending fee storage. At the following oracle version, the old fee will be restored.

It should be noted that even when there are pre-positions in the product at the time of product owner attack, the attack might still succeed as long as a new Oracle version is available at the time of user invoking openTake or openMake.

Impact

Users targeted by a malicious product owner will lose funds.

Code Snippet

https://github.com/equilibria-xyz/perennial-mono/blob/b06d5145db62a312dd88dfcafef0f8e2166c5e32/packages/perennial/contracts/product/UParamProvider.sol#L148-L152

https://github.com/equilibria-xyz/perennial-mono/blob/b06d5145db62a312dd88dfcafef0f8e2166c5e32/packages/perennial/contracts/product/UParamProvider.sol#L125-L128

Tool used

Manual Review

Recommendation

(1) Cap maker and taker fee to a reasonable level. (2) Consider adding timelock delay to ensure there is ample time of notice to users when fees are updated.