Omniscia Symbiosis Finance Audit

MetaRouterV2 Manual Review Findings

MetaRouterV2 Manual Review Findings

MRV-01M: Inexistent Validation of Calldata Slots

Description:

The low level assembly writes to the two call datas are meant to fill in the value of a particular argument for the external call, however, no validation is performed on the calldata that can lead to out-of-bounds writes in the blocks as well as generally unexpected behaviour.

Example:

contracts/metarouter/MetaRouterV2.sol
53uint256 finalSwapAmountIn = secondSwapAmountIn;
54if (_metarouteTransaction.secondSwapCalldata.length != 0) {
55    bytes memory secondSwapCalldata = _metarouteTransaction.secondSwapCalldata;
56

57    assembly {
58        mstore(add(secondSwapCalldata, 100), secondSwapAmountIn)
59    }
60

61    IERC20(_metarouteTransaction.approvedTokens[approvedTokensLength - 2]).approve(
62        _metarouteTransaction.secondDexRouter,
63        secondSwapAmountIn
64    );
65

66    (bool secondSwapSuccess,) = _metarouteTransaction.secondDexRouter.call(
67        secondSwapCalldata
68    );
69    require(secondSwapSuccess, "MetaRouterV2: second swap failed");
70

71    finalSwapAmountIn = IERC20(
72        _metarouteTransaction.approvedTokens[approvedTokensLength - 1]
73    ).balanceOf(address(this));
74}
75

76IERC20(_metarouteTransaction.approvedTokens[approvedTokensLength - 1]).approve(
77    _metarouteTransaction.relayRecipient,
78    finalSwapAmountIn
79);
80

81bytes memory otherSideCalldata = _metarouteTransaction.otherSideCalldata;
82assembly {
83    mstore(add(otherSideCalldata, 100), finalSwapAmountIn)
84}

Recommendation:

We advise the calldata arguments to be validated by at least mandating they are of a particular length.

Alleviation:

After consideration of our exhibit & with the help of an external party, the Symbiosis Finance team identified a potential attack vector based on allowances set to the contract that arbitrary calls could exploit. The Symbiosis Finance team introduced the concept of a gateway contract that is meant to instead be set an allowance for by external users preventing the arbitrary calls performed by the MetaRouterV2 contract to be able to tap into allowances set for it. Additionally, the two arbitrary calls performed now cannot have the gateway contract as a target thereby completely nullifying any attack vector that would affect user funds and rendering the contract secure. After additional discussion with the Symbiosis Finance team, we concluded that malicious data stacks for the linked assembly blocks would only affect the caller and would not pose a threat to other users or the network's state. As a result, this exhibit is considered dealt with.

MRV-02M: Arbitrary Approvals

Description:

The contract performs arbitrary approve invocations which allow crafted payloads to extract any funds at rest within the contract.

Example:

contracts/metarouter/MetaRouterV2.sol
18function metaRouteV2(
19    MetaRouteStructs.MetaRouteTransactionV2 memory _metarouteTransaction
20) external payable {
21    uint256 firstSwapValue;
22    uint256 approvedTokensLength = _metarouteTransaction.approvedTokens.length;
23

24    if (!_metarouteTransaction.nativeIn) {
25        TransferHelper.safeTransferFrom(
26            _metarouteTransaction.approvedTokens[0],
27            _msgSender(),
28            address(this),
29            _metarouteTransaction.amount
30        );
31    }
32

33    uint256 secondSwapAmountIn = _metarouteTransaction.amount;
34    if (_metarouteTransaction.firstSwapCalldata.length != 0) {
35        if (!_metarouteTransaction.nativeIn) {
36            IERC20(_metarouteTransaction.approvedTokens[0]).approve(
37                _metarouteTransaction.firstDexRouter,
38                _metarouteTransaction.amount
39            );
40        }
41

42        (bool firstSwapSuccess,) = _metarouteTransaction
43        .firstDexRouter
44        .call{value : msg.value}(_metarouteTransaction.firstSwapCalldata);
45

46        require(firstSwapSuccess, "MetaRouterV2: first swap failed");
47

48        secondSwapAmountIn = IERC20(
49            _metarouteTransaction.approvedTokens[1]
50        ).balanceOf(address(this));
51    }
52

53    uint256 finalSwapAmountIn = secondSwapAmountIn;
54    if (_metarouteTransaction.secondSwapCalldata.length != 0) {
55        bytes memory secondSwapCalldata = _metarouteTransaction.secondSwapCalldata;
56

57        assembly {
58            mstore(add(secondSwapCalldata, 100), secondSwapAmountIn)
59        }
60

61        IERC20(_metarouteTransaction.approvedTokens[approvedTokensLength - 2]).approve(
62            _metarouteTransaction.secondDexRouter,
63            secondSwapAmountIn
64        );
65

66        (bool secondSwapSuccess,) = _metarouteTransaction.secondDexRouter.call(
67            secondSwapCalldata
68        );
69        require(secondSwapSuccess, "MetaRouterV2: second swap failed");
70

71        finalSwapAmountIn = IERC20(
72            _metarouteTransaction.approvedTokens[approvedTokensLength - 1]
73        ).balanceOf(address(this));
74    }
75

76    IERC20(_metarouteTransaction.approvedTokens[approvedTokensLength - 1]).approve(
77        _metarouteTransaction.relayRecipient,
78        finalSwapAmountIn
79    );
80

81    bytes memory otherSideCalldata = _metarouteTransaction.otherSideCalldata;
82    assembly {
83        mstore(add(otherSideCalldata, 100), finalSwapAmountIn)
84    }
85

86    (bool otherSideCallSuccess,) = _metarouteTransaction.relayRecipient
87                                        .call(otherSideCalldata);
88    require(otherSideCallSuccess, "MetaRouterV2: other side call failed");
89}

Recommendation:

While funds are not expected to remain at rest, it is still advisable to perform approvals only to authorized exchanges and to validate that a swap was indeed made before performing the final transaction. In general, the router should identify the amounts it received via the return arguments of the swaps rather than rely on dynamic balanceOf invocations.

Alleviation:

The Symbiosis Finance team stated that given the context of the contract any allowance will not pose a threat to other users or the network and as such they opt to not remediate it.

MRV-03M: Ill-Advised Allowance Pattern

Description:

The linked code performs an "infinity" allowance to the router it is meant to interact with, a programming paradigm that is advised against.

Example:

contracts/metarouter/MetaRouterV2.sol
137function _swap(
138    address _token,
139    uint256 _amount,
140    address _router,
141    bytes memory _swapCalldata,
142    uint256 _offset
143) internal returns (bool success) {
144    if (IERC20(_token).allowance(address(this), _router) < _amount) {
145        IERC20(_token).approve(
146            _router,
147            type(uint256).max
148        );
149    }
150

151    assembly {
152        mstore(add(_swapCalldata, _offset), _amount)
153    }
154

155    (success, ) = _router.call(_swapCalldata);
156}

Recommendation:

We advise the allowance to be set to exactly the value necessary to avoid potential complications due to unspent allowance.

Alleviation:

The Symbiosis Finance team stated that given the context of the contract any allowance will not pose a threat to other users or the network and as such they opt to not remediate it.

MRV-04M: Improper receive Function

Description:

The MetaRouterV2 contract is able to receive native assets, however, no function exists in the contract that utilizes funds received as an argument.

Example:

contracts/metarouter/MetaRouterV2.sol
15receive() external payable {}

Recommendation:

Presumably, this function was introduced to allow native outputs in the swaps the contract performs, however, no outward native asset transfer is performed by the contract that utilizes converted or existing (address(this).balance) funds. As a result, we advise the function to be omitted from the contract.

Alleviation:

The receive function has been omitted from the codebase as per our recommendation.

Prev

BridgeV2.sol (BV2-M)

Next

Portal.sol (POR-M)

On this page