MetaTransactionsHandlerFacet Manual Review Findings

MTH-01M: Account Agnostic Nonce System

Type	Severity	Location
Logical Fault		MetaTransactionsHandlerFacet.sol:L173, L218

Description:

The cryptographic system of the MetaTransactionsHandlerFacet does not utilize an account-specific nonce thus paving way to race conditions that allow selectively cancelling meta transactions at will.

Impact:

It is possible to hi-jack meta-transaction execution flows that can have varying degrees of impact ranging from locked funds to complete transaction workflow failures.

Example:

Copy
168function validateTx(
169    string calldata _functionName,
170    bytes calldata _functionSignature,
171    uint256 _nonce
172) internal view {
173    require(!protocolMetaTxInfo().usedNonce[_nonce], NONCE_USED_ALREADY);
174
175    bytes4 destinationFunctionSig = convertBytesToBytes4(_functionSignature);
176    require(destinationFunctionSig != msg.sig, INVALID_FUNCTION_SIGNATURE);
177
178    bytes4 functionNameSig = bytes4(keccak256(abi.encodePacked(_functionName)));
179    require(destinationFunctionSig == functionNameSig, INVALID_FUNCTION_NAME);
180}

Recommendation:

We advise an account-based nonce to be utilized instead that ensures the transactions are executed in the sequence they are meant to and that the sequence cannot be hijacked as currently a malicious user can detect a transaction that they wish to cancel and submit their own no-op meta-transaction with the same nonce thus invalidating it.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The contract's code has been updated to introduce an additional address based key to the usedNonce mapping thus ensuring that the nonce system is bound to each account and cannot be hi-jacked by external account actions.