Polynomial Protocol contest - kaden's results

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L494-L521

Vulnerability details

Impact

Fees received by the LiquidityPool contract when a short position is opened are unaccounted for and end up permanently locked in the contract, incurring a loss to liquidity providers.

Proof of Concept

When opening and closing short positions, the LiquidityPool requires either a fee to be transferred in from the user account or withheld in the case that sUSD is being transferred from the contract to the user. Consider for example how fees must be transferred from the user in openLong:

// openLong() - LiquidityPool.sol:L441-444
uint256 fees = orderFee(int256(amount));
totalCost = tradeCost + fees;

SUSD.safeTransferFrom(user, address(this), totalCost);

Since the contract is taking a fee, it's necessary to update the corresponding totalFunds state variable to reflect the collected fee:

// openLong() - LiquidityPool.sol:L453
totalFunds += feesCollected - externalFee;

In openShort, however, we deduct a fee from the amount being transferred to the user as expected:

// openShort() - LiquidityPool.sol:L505-508
uint256 fees = orderFee(-int256(amount));
totalCost = tradeCost - fees;

SUSD.safeTransfer(user, totalCost);

But there is no corresponding update to totalFunds.

Since we can't withdraw beyond totalFunds (the following will underflow and revert):

// withdraw() - LiquidityPool.sol:255
totalFunds -= susdToReturn;

All fees collected in openShort are not accounted for and permanently locked in the contract. Since these fees would go to the liquidity providers, this is considered a loss of funds for users.

Recommended Mitigation Steps

Include the necessary update to totalFunds in openShort:

totalFunds += feesCollected - externalFee;