0xean
commented
2 years ago The protocol is meant to be permission-less and all of the solutions outlined by the warden are permissioned. Yes, this does require the correct configuration of pairs and yes users will have to verify that the pair they are interacting with is correct. We have UI flows that do both of these pieces.
Handle
sirhashalot
Vulnerability details
Impact
When a new exchange is created, there are no checks to verify whether the base token value entered by the user is a quote token or vice versa. A user could create an exchange with 2 quote tokens, 2 base tokens, or the base and quote tokens in reversed positions. Because the math for this DEX assumes the quote and base tokens in certain positions, with different MathLib.sol functions for different cases, breaking this assumption means the math underlying this DEX can produce incorrect values and result in unexpected scenarios that can allow users to extract value from such pools.
Proof of Concept
The project documentation describes the base token and quote token as distinct from one another:
At the same time, the ExchangeFactory.sol
createNewExchange()function never checks whether the _baseToken input parameter is a base token (and not a quote token) and whether the _quoteToken input parameter is a quote token (and not a base token). Because no checks are performed, a user can create a new exchange with any address in the _baseToken and _quoteToken input parameters, leading to tokens being incorrectly categorized as base tokens or quote tokens.The math in MathLib.sol is highly dependent on the assumptions of base token and quote tokens being sorted properly. If a pool exists where both tokens are quote tokens, the results calculated in MathLab.sol will be wrong.
Recommended Mitigation Steps
A simple fix would be to add the onlyOwner modifier to the
createNewExchange()function, since the owner can be a trusted party. A more complex approach to allow any user to create a new exchange could maintain a whitelist of acceptable base tokens and quote tokens. By checking whether the _baseToken input parameter is in the case token whitelist and doing the same for the _quoteToken input parameter, thecreateNewExchange()function could safely avoid creating an exchange where the tokens meet the DEX assumptions of base token and quote token properties.