SpotHedgeBaseMaker uses the wrong oracle for non-evm/non-erc20 markets

Summary

Markets are priced using a 'base' token, which can be something like Eth, or BTC. These assets have prices which are different from their ERC20 analogs (e.g. the price of BTC is not the same as the price of WBTC, because the company that maintains WBTC can go bankrupt). The SpotHedgeBaseMaker uses the oracle of the base token during deposit/withdraw, rather than the oracle for the actual token being deposited.

Vulnerability Detail

LP shares are valued based on the total value accrued to LP, which consists of gains made in the collateral token (e.g. via borrow fees), as well as the value of the deposited tokens. LP shares are redeemed first from the deposited tokens, and once those run out, they're paid using the vault's collateral tokens, using the market's base token's oracle, not the oracle of the deposited token.

Impact

If the token dep-pegs, an attacker can deposit a lot of the deposit token for more shares than they should get. They can then wait for other participants to exit the market so that all originally-deposited tokens are swapped back from the collateral, leaving a pool of gains from fees, and a pool of deposited tokens. At that point, anyone who redeems their shares will get the tokens they deposited, plus some of the attacker's tokens as their portion of fees. Once all other LPs have withdrawn, the attacker can redeem their shares for solely the collateral token, which will be worth more than the base token they deposited.

Code Snippet

The price oracle used to value the deposited tokens is the market's oracle's, not the LP deposit/withdraw token's oracle (a different marketId for the SHBM and the perp can't be used):

// File: src/maker/SpotHedgeBaseMaker.sol : SpotHedgeBaseMaker.price   #1

769        function _getPrice() internal view returns (uint256) {
770            IAddressManager addressManager = getAddressManager();
771 @>         (uint256 price, ) = addressManager.getPythOracleAdapter().getPrice(
772                addressManager.getConfig().getPriceFeedId(_getSpotHedgeBaseMakerStorage().marketId)
773            );
774            return price;
775:       }

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/maker/SpotHedgeBaseMaker.sol#L769-L775

When the market has been wound down and there are no more deposited (spot) tokens, the remaining value is paid out using collateral using the wrong oracle:

// File: src/maker/SpotHedgeBaseMaker.sol : SpotHedgeBaseMaker.withdraw()   #2

331            if (withdrawnBaseAmount > spotBaseBalance) {
332                if (vault.getPositionSize(_getSpotHedgeBaseMakerStorage().marketId, maker) != 0) {
333                    revert LibError.NotEnoughSpotBaseTokens(withdrawnBaseAmount, spotBaseBalance);
334                } else {
335 @>                 withdrawnQuoteAmount = (withdrawnBaseAmount - spotBaseBalance).mulWad(price).formatDecimals(
336                        _getSpotHedgeBaseMakerStorage().baseTokenDecimals,
337                        _getSpotHedgeBaseMakerStorage().quoteTokenDecimals
338                    );
339                    withdrawnBaseAmount = FixedPointMathLib.min(withdrawnBaseAmount, spotBaseBalance);
340                }
341:           }

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/maker/SpotHedgeBaseMaker.sol#L331-L341