Reversion in _executeOrderV3 Due to Uninitialized Quote Token Mappings in _validateQuoteTokenAddress Function

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/markets/perp/PerpMarketV1.sol#L167
_validateQuoteTokenAddress](https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/base/BaseMarketUpgradable.sol#L152

Vulnerability details

Impact

The issue in the _executeOrderV3 function, where the _validateQuoteTokenAddress might cause reversion due to outdated or uninitialized quote token mappings.

Proof of Concept

Consider a scenario where a new trading pair is added to the platform, but the quote token address mapping is not updated:

Pair Addition: A new pair, say ETH/USDT, is added with a pair ID of 101.

User Transaction: A user tries to execute a trade on this new pair using _executeOrderV3.

Mapping Check: The _validateQuoteTokenAddress checks _quoteTokenMap[101], which is uninitialized (address(0)) because it was never updated after the pair was added.

Result: The transaction reverts with "Invalid token address for pair", even though the user's order was valid.

Tools Used

Manual Review

Recommended Mitigation Steps

the _validateQuoteTokenAddress function to automatically update the quote token address if it is found to be uninitialized. This ensures that the mapping is always current.

function _validateQuoteTokenAddress(uint256 pairId, address entryTokenAddress) internal {
++   if (_quoteTokenMap[pairId] == address(0)) {
++      _quoteTokenMap[pairId] = _getQuoteTokenAddress(pairId);
    }
    require(_quoteTokenMap[pairId] != address(0) && entryTokenAddress == _quoteTokenMap[pairId], "Invalid token address for pair");
}

Assessed type

Context

[0xAbhay]

Hey @alex-ppg, thanks for judging please take a look here thank you.

Vulnerability Details

Issue: In the _executeOrderV3 function, the _validateQuoteTokenAddress function can cause transactions to fail if the quote token mappings are outdated or uninitialized.

Impact: This can prevent valid user trades from executing, leading to failed transactions.

Proof of Concept

1. New Pair Addition: Suppose a new trading pair, like ETH/USDT with pair ID 101, is added to the platform.
2. User Transaction: A user tries to execute a trade on this new pair.
3. Mapping Check: The _validateQuoteTokenAddress function checks _quoteTokenMap[101], which is uninitialized and returns address(0) because the mapping wasn't updated.
4. Result: The transaction fails with "Invalid token address for pair," even though the trade is valid.

Recommended Mitigation

To fix this, the _validateQuoteTokenAddress function should automatically update the quote token address if it is uninitialized. Here’s the suggested code change:

function _validateQuoteTokenAddress(uint256 pairId, address entryTokenAddress) internal {
    if (_quoteTokenMap[pairId] == address(0)) {
        _quoteTokenMap[pairId] = _getQuoteTokenAddress(pairId);
    }
    require(_quoteTokenMap[pairId] != address(0) && entryTokenAddress == _quoteTokenMap[pairId], "Invalid token address for pair");
}

This ensures the quote token mapping is always current and prevents valid transactions from failing.

Even though we have an updateQuoteTokenMap function, there is no incentive to use it promptly. Therefore, delays in updating the mapping can occur. My recommendation is fine because it ensures the mapping is updated without needing to call updateQuoteTokenMap separately.

[alex-ppg]

Hey @0xAbhay, thanks for contributing to the PJQA process! This represents a validation repository finding and as such was not evaluated by me directly.

This submission cannot be considered an HM vulnerability, and cannot be considered a QA-level flaw either as an acceptable alternative is already available in the contract itself.