Root + Impact

Withdrawing core orderbook tokens still possible if added manually after deployment.

Description

• No core orderbook tokens should be withdrawable from the contract using emergencyWithdrawERC20() as stipulated in line 283.

• Nonetheless it is still possible to withdraw core orderbook tokens if those are added after construction/deployment using the setAllowedSellToken() function. emergencyWithdrawERC20() only checks for the 4 core tokens defined at construction/deployment of the contract which are : WETH, WBTC, WSOL & USDC.

Risk

Likelihood: LOW

• Those are functions only executable by the Owner of the contract, we can suppose that the owner knows what he does but still there is a chance he could forget or be mistaken.
  One example could be multiple tokens are added through setAllowedSellToken() and one day a user send tokens by mistake in the contract, contact the owner to get back those tokens. This specific token is part of the core tokens but the owner just forgot about this specific token being added to the list. He remembers that there is a security in place to not be able to touch core tokens so he just performs the 'Emergency Withdraw' and now the contract is broken !

Impact: High

• Loss of funds for the user(s)

Proof of Concept

1) Add wAVAX as an allowed token to sell.


2) Try to withdraw wAVAX, the function won't revert.
Those checks line 280 & 281 were set to prevent "withdrawing core order book tokens" as mentioned in line 283, but the emergencyWithdrawErc20() function only checks for the wBTC, wETH, wSOL and iUSDC, not wAVAX added manually.

Recommended Mitigation

Should not use a mapping to keep track on 'allowed sell tokens' but an array and do a check that loops all the array and forbid withdrawing those tokens.