-
Notifications
You must be signed in to change notification settings - Fork 34
M01. Double invocation of rule-engine compliance hook in cross-chain overrides #354
Description
Version concerned: CMTAT v3.1.0
Summary
In contracts/modules/4_CMTATBaseERC20CrossChain.sol, the functions _mintOverride, _burnOverride, and _minterTransferOverride each called CMTATBaseRuleEngine._checkTransferred directly before delegating to the corresponding CMTATBaseCommon override. Since CMTATBaseCommon already calls _checkTransferred internally, the external IRuleEngine.transferred hook was being executed twice per operation.
This was identified by Ackee Blockchain Security as findings M1, M2, and M3 in the Wake Arena AI Repor
Impact
- Severity: Medium
- Rule engines that decrement allowances, track quotas, or enforce per-operation rate limits will apply their side effects twice for a single mint, burn, or minter-initiated transfer.
- This can cause valid operations to revert unexpectedly, exhaust quotas prematurely, or produce inconsistent compliance accounting.
- Gas cost is unnecessarily inflated by the duplicate external call.
Affected functions
| Function | File |
|---|---|
_mintOverride |
contracts/modules/4_CMTATBaseERC20CrossChain.sol |
_burnOverride |
contracts/modules/4_CMTATBaseERC20CrossChain.sol |
_minterTransferOverride |
contracts/modules/4_CMTATBaseERC20CrossChain.sol |
Root cause
Each override explicitly called CMTATBaseRuleEngine._checkTransferred(...) and then delegated to its CMTATBaseCommon counterpart, which itself calls _checkTransferred again before executing the operation.
Before (vulnerable)
function _mintOverride(address account, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20MintModuleInternal)
{
CMTATBaseRuleEngine._checkTransferred(address(0), address(0), account, value); // ← redundant
CMTATBaseCommon._mintOverride(account, value); // already calls _checkTransferred internally
}
function _burnOverride(address account, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20BurnModuleInternal)
{
CMTATBaseRuleEngine._checkTransferred(address(0), account, address(0), value); // ← redundant
CMTATBaseCommon._burnOverride(account, value); // already calls _checkTransferred internally
}
function _minterTransferOverride(address from, address to, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20MintModuleInternal)
{
CMTATBaseRuleEngine._checkTransferred(address(0), from, to, value); // ← redundant
CMTATBaseCommon._minterTransferOverride(from, to, value); // already calls _checkTransferred internally
}After (fixed)
function _mintOverride(address account, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20MintModuleInternal)
{
// _checkTransferred is called by CMTATBaseCommon._mintOverride
CMTATBaseCommon._mintOverride(account, value);
}
function _burnOverride(address account, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20BurnModuleInternal)
{
// _checkTransferred is called by CMTATBaseCommon._burnOverride
CMTATBaseCommon._burnOverride(account, value);
}
function _minterTransferOverride(address from, address to, uint256 value)
internal virtual override(CMTATBaseCommon, ERC20MintModuleInternal)
{
// _checkTransferred is called by CMTATBaseCommon._minterTransferOverride
CMTATBaseCommon._minterTransferOverride(from, to, value);
}Fix
Removed the direct calls to CMTATBaseRuleEngine._checkTransferred from all three overrides. The single authoritative compliance check is now performed exclusively inside the CMTATBaseCommon parent overrides.
Fixed in: commit 0635d82 – "Wake: remove redundant call" (v3.2.0)