During the audit of the MultiInvoker.sol contract, a potential security issue was discovered concerning the _marketSettle, _executeOrder, and _cancelOrder functions. These functions accept an IMarket interface that may not necessarily be created by MarketFactory, which could result in unforeseen behaviour or vulnerabilities in the contract. While the current design restricts external access to these functions, the absence of validation for the origin of the IMarket interface poses risks if the contract's architecture changes or if external contracts gain interaction privileges.
The lack of validation for the origin of the IMarket interface may allow unauthorized contracts to interact with critical functions, potentially leading to loss of funds or other unintended consequences.
Code Snippet
function _executeOrder(address account, IMarket market, uint256 nonce) internal {
// Function implementation...
}
function _marketSettle(IMarket market, address account) private {
// Function implementation...
}
function _cancelOrder(address account, IMarket market, uint256 nonce) internal {
// Function implementation...
}
Tool used
Manual Review
Recommendation
Implement a validation check within the _marketSettle, _executeOrder, and _cancelOrder functions to ensure that the provided IMarket instance is created by MarketFactory.
Using the modifier shown below should be enough:
modifier isMarketInstance(IMarket market) {
if (!marketFactory.instances(market)) revert MultiInvokerInvalidInstanceError();
_;
}
Low 02 IMarket Instance
Low/Info issue submitted by 1337web3
Summary
During the audit of the
MultiInvoker.sol
contract, a potential security issue was discovered concerning the_marketSettle
,_executeOrder
, and_cancelOrder
functions. These functions accept anIMarket
interface that may not necessarily be created byMarketFactory
, which could result in unforeseen behaviour or vulnerabilities in the contract. While the current design restricts external access to these functions, the absence of validation for the origin of theIMarket
interface poses risks if the contract's architecture changes or if external contracts gain interaction privileges.Vulnerability Detail
Affected Functions:
_executeOrder(address account, IMarket market, uint256 nonce)
_marketSettle(IMarket market, address account)
_cancelOrder(address account, IMarket market, uint256 nonce)
https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L368-L408
https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L462-L464
https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L446-L449
Impact
The lack of validation for the origin of the
IMarket
interface may allow unauthorized contracts to interact with critical functions, potentially leading to loss of funds or other unintended consequences.Code Snippet
Tool used
Manual Review
Recommendation
Implement a validation check within the
_marketSettle
,_executeOrder
, and_cancelOrder
functions to ensure that the providedIMarket
instance is created by MarketFactory. Using the modifier shown below should be enough: