Organization
- @coinbase
Engagement Type
Cantina Reviews
Period
-
Repositories
Findings
Low Risk
7 findings
6 fixed
1 acknowledged
Informational
7 findings
7 fixed
0 acknowledged
Low Risk7 findings
ERC-3009 typehash constants are private, diverging from specification's public visibility requirement
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
EIP-3009 defines three typehash constants as part of the contract's public interface:
bytes32 public constant TRANSFER_WITH_AUTHORIZATION_TYPEHASH = 0x7c7c6cdb67a18743f49ec6fa9b35f50d52ed05cbed4cc592e13b44501c1a2267; bytes32 public constant RECEIVE_WITH_AUTHORIZATION_TYPEHASH = 0xd099cc98ef71107a616c4f0f941f04c322d8e254fe26b3c6668db87aae413de8; bytes32 public constant CANCEL_AUTHORIZATION_TYPEHASH = 0x158b0a9edf7a828aad02f63cd515c68ef2f50ba807396f6d12842833a1597429;However, the implementation in
ERC3009Upgradeable.soldeclares all three as private. This results in a slight non-compliance with the standard. Hence any tooling or integration that expects to query the typehashes on-chain (as the EIP mandates) will fail.Recommendation
Change the visibility from private to public on all three constants:
- bytes32 private constant TRANSFER_WITH_AUTHORIZATION_TYPEHASH = + bytes32 public constant TRANSFER_WITH_AUTHORIZATION_TYPEHASH = 0x7c7c6cdb67a18743f49ec6fa9b35f50d52ed05cbed4cc592e13b44501c1a2267; - bytes32 private constant RECEIVE_WITH_AUTHORIZATION_TYPEHASH = + bytes32 public constant RECEIVE_WITH_AUTHORIZATION_TYPEHASH = 0xd099cc98ef71107a616c4f0f941f04c322d8e254fe26b3c6668db87aae413de8; - bytes32 private constant CANCEL_AUTHORIZATION_TYPEHASH = + bytes32 public constant CANCEL_AUTHORIZATION_TYPEHASH = 0x158b0a9edf7a828aad02f63cd515c68ef2f50ba807396f6d12842833a1597429;Function currentLimit() reverts with unhandled panic for unconfigured accounts
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
Function
currentLimit()is a public view function intended to return the available capacity for a given (key, account) pair. When called for an account that has no rate-limit configuration (i.e., all struct fields are zero-initialized), it reverts with a raw Panic(0x12) (division by zero) instead of returning 0 or reverting with a descriptive error.PoC
function test_audit() external { // Query an account with no rate-limit configuration stablecoin.currentLimit(stablecoin.MINT_RATE_LIMIT_KEY(), address(101)); // Result: panic: division or modulo by zero (0x12) }Test Output: [FAIL: panic: division or modulo by zero (0x12)] test_audit() (gas: 22799)
Recommendation
- Add an early return for unconfigured accounts, consistent with how _consumeLimit guards the same path:
function currentLimit(bytes32 key, address account) public view virtual returns (uint256) { RateLimitConfig storage config = _getRateLimitLayout().limits[key][account]; + if (config.interval == 0) return 0; uint256 elapsed = block.timestamp - config.lastConsumed; uint256 replenishmentAmount = Math.mulDiv(elapsed, config.limit, config.interval); return Math.min(config.remaining + replenishmentAmount, config.limit); }This returns 0 for unconfigured accounts (which is semantically correct as they have no capacity) without breaking existing callers.
- Or consider returning early with a "RateLimitNotConfigured" error for unconfigured accounts.
Function _configureRateLimit() resets remaining capacity to full limit on reconfiguration
State
- Acknowledged
Severity
- Severity: Low
Submitted by
Sujith S
Description
Function
_configureRateLimit()overwrites the entireRateLimitConfigstruct, resetting remaining to the full limit regardless of how much capacity the account has already consumed.This function is called by both
grantMinterRoleWithLimit()(admin-only) andconfigureMinter()(rate-limit-role). Any reconfiguration like even a minor parameter adjustment like changing interval fully replenishes the account's capacity.Proof of Concept
function test_audit_reconfigure_resets_remaining() external { // Minter starts with 1_000_000e6 capacity uint256 limitBefore = stablecoin.currentMintLimit(minter); assertEq(limitBefore, 1_000_000e6); // Minter consumes 900_000e6 (90% of capacity) vm.prank(minter); stablecoin.mint(user1, 900_000e6); uint256 limitAfterMint = stablecoin.currentMintLimit(minter); assertEq(limitAfterMint, 100_000e6); // Admin reconfigures the minter (e.g. just to change interval) // but remaining resets to the full limit vm.prank(admin); stablecoin.grantMinterRoleWithLimit(minter, 1_000_000e6, 2 days); uint256 limitAfterReconfig = stablecoin.currentMintLimit(minter); // Remaining is fully refilled despite 90% being consumed moments ago assertEq(limitAfterReconfig, 1_000_000e6); // Minter can now mint the full limit again and rate limit was bypassed vm.prank(minter); stablecoin.mint(user1, 1_000_000e6); assertEq(stablecoin.balanceOf(user1), 1_900_000e6); }Test Output: [PASS] test_audit_reconfigure_resets_remaining() (gas: 132304)
Recommendation
Preserve the current consumed state when reconfiguring. Compute the current available capacity and carry it forward
proportionally, or at minimum cap remaining to the lesser of the new limit and the current available capacity.Coinbase: Added documentation explaining this behaviour.
Cantina: Acknowledged.
PAUSE_ROLE controls both pause and unpause, preventing privilege separation
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
Both
pause()andunpause()are gated by the samePAUSE_ROLE. In practice, pause and unpause have very different risk profiles:Pause is an emergency action, typically triggered by automated off-chain watchers or hot wallets that monitor for exploits, anomalous transfers, or oracle failures. Speed is critical and the key must be readily available.
Unpause is a recovery action that should require stronger assurance that the threat has been resolved. It benefits from a higher-privilege signer (multisig, timelock, or cold wallet).
Recommendation
Introduce a separate
UNPAUSE_ROLE:+ bytes32 public constant UNPAUSE_ROLE = keccak256("UNPAUSE_ROLE"); - function unpause() external onlyRole(PAUSE_ROLE) { + function unpause() external onlyRole(UNPAUSE_ROLE) { _unpause(); }This allows operators to grant
PAUSE_ROLEto a hot wallet or monitoring bot for rapid response, while restricting
UNPAUSE_ROLEto a multisig or governance contract that confirms the threat is resolved before resuming operations.Both roles can still be assigned to the same address if separation isn't desired.
Functions approve() and permit() bypass blocklist enforcement
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
The Stablecoin contract enforces its blocklist exclusively in the
_update()hook, which gates all token movement (transfers, mints, burns). However, approve and permit modify allowances through_approve(), which does not pass through_update().Neither path checks whether
msg.sender,owner, orspenderis blocklisted. A blocklisted address can freely callapprove(), and anyone can submit a pre-signedpermit()for a blocklisted signer and the ERC-2612 nonce is consumed and the allowance is set.Recommendation
Consider overriding the
_approve()internal function to include blocklist checks.Unoverridden renounceOwnership() bypasses two-step ownership protection in TwoStepUpgradeableBeacon
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
TwoStepUpgradeableBeacon inherits from both
UpgradeableBeaconandOwnable2Stepto enforce a two-step (propose + accept) ownership transfer pattern. The contract explicitly overridestransferOwnership()and_transferOwnership()to route through Ownable2Step's two-step logic, and the inline documentation states:"Ownership does not change until the pending owner explicitly calls acceptOwnership, preventing an erroneous address from permanently locking beacon upgrades."
However,
Ownable2Stepdoes not overriderenounceOwnership(). The single-stepOwnable.renounceOwnership()remains publicly callable by the current owner and immediately sets the owner toaddress(0)with no pending step, no confirmation, and no recovery path.This directly contradicts the contract's stated security invariant.
Recommendation
Override
renounceOwnership()inTwoStepUpgradeableBeaconto revert unconditionally, making it impossible to accidentally or intentionally orphan the beacon:/// @notice Renouncing ownership is disabled to prevent permanently locking beacon upgrades.function renounceOwnership() public pure override { revert("TwoStepUpgradeableBeacon: renounce disabled");}If there is a legitimate need to allow renunciation in the future, it should be implemented with the same two-step confirmation pattern used for transfers (e.g.,
proposeRenounce()+confirmRenounce()with a timelock), ensuring consistency with the contract's design intent.Separate BLOCKLIST_ROLE into BLOCKLIST_ROLE and UNBLOCKLIST_ROLE
State
Severity
- Severity: Low
Submitted by
Sujith S
Description
Function
updateBlocklistStatus()handles both blocklisting and un-blocklisting addresses and is gated by a singleBLOCKLIST_ROLE. In practice, these two operations have very different risk profiles:-
Blocklisting is a protective action, typically triggered by compliance systems, automated monitoring bots, or hot wallets that detect sanctioned addresses, stolen funds, or illicit activity. Speed is critical and the key must be readily available to freeze assets before they can be moved.
-
Un-blocklisting is a restorative action that re-enables an address to transfer tokens. It should require stronger assurance that the compliance concern has been resolved (e.g., false positive confirmed, legal review completed, law enforcement clearance). Granting the same hot wallet or bot that blocklists the ability to also un-blocklist introduces risk that a compromised bot key could silently un-blocklist a sanctioned address.
Recommendation
Introduce a separate
UNBLOCKLIST_ROLEand split the single updateBlocklistStatus function into two distinct functions:+ bytes32 public constant UNBLOCKLIST_ROLE = keccak256("UNBLOCKLIST_ROLE"); - function updateBlocklistStatus(address account, bool blocklisted) external onlyRole(BLOCKLIST_ROLE) { - _updateBlocklistStatus({account: account, blocklisted: blocklisted}); - } + function blocklist(address account) external onlyRole(BLOCKLIST_ROLE) { + _updateBlocklistStatus({account: account, blocklisted: true});+ } + + function unblocklist(address account) external onlyRole(UNBLOCKLIST_ROLE) {+ _updateBlocklistStatus({account: account, blocklisted: false}); + }This allows operators to:
- Grant
BLOCKLIST_ROLEto a hot wallet or compliance bot for rapid response to illicit activity. - Restrict
UNBLOCKLIST_ROLEto a multisig, compliance committee, or governance contract that confirms the issue is resolved before re-enabling the address.
Both roles can still be assigned to the same address if separation isn't desired and the change is purely additive.
Informational7 findings
Unused import of IBeacon in MutableBeaconProxy.sol
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
The
MutableBeaconProxycontract importsIBeaconfrom@openzeppelin/contracts/proxy/beacon/IBeacon.sol, but the identifier is never used within the contract. The contract only usesBeaconProxyandERC1967Utils.Recommendation
Remove the unused import statement to improve code clarity and eliminate dead dependencies:
- import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol";LimitReplenished event is declared but never emitted
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
RateLimit.soldeclares theLimitReplenishedevent but never emits it anywhere in the contract. Replenishment occurs implicitly within_consumeLimit(), wherecurrentLimit()computes the replenished capacity and the result is written directly toconfig.remaining. However, onlyLimitConsumedis emitted and the replenishment amount is silently folded into the updated remaining value with no corresponding eventRecommendation
Either emit the event in
_consumeLimit()when replenishment occurs, or remove the dead declaration.Stale comment references uint192 instead of uint216
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
The safety comment at
RateLimit.solreferences the wrong bit-width:// Safe: amount <= currentLimit_ is enforced above, and currentLimit_ fits in uint192. config.remaining = uint216(currentLimit_ - amount);The actual type of remaining and the cast is
uint216, notuint192. The safety reasoning is correct andcurrentLimit_is
capped atconfig.limitwhich isuint216but the comment is stale.Recommendation
-// Safe: amount <= currentLimit_ is enforced above, and currentLimit_ fits in uint192. +// Safe: amount <= currentLimit_ is enforced above, and currentLimit_ fits in uint216.Custom functions use msg.sender instead of _msgSender()
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
OpenZeppelin's ERC-20 functions resolve the caller via
_msgSender(), which supports overriding for meta-transaction patterns like ERC-2771. But every custom function inStablecoin.soluses rawmsg.senderinstead.Recommendation
Replace
msg.senderwith_msgSender()in all custom functions for consistency with the inherited OZ pattern.Coinbase: Fixed in a766dc15
Cantina: Verified fix. The team removed ERC 2771 support entirely by overriding
_msgSender()function.Add more parameters to StablecoinDeployed event
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
The
StablecoinDeployedevent inStablecoinFactory.solcurrently only emits the deployed proxy address. However, the deploy function receives five additional parameters includingname,symbol,decimals,stablecoinAdmin, andsaltnone of which are included in the emitted event.This means off-chain consumers (indexers, subgraphs, dashboards, monitoring tools) cannot determine the token's configuration from the event alone and must make additional RPC calls to the deployed contract to retrieve this information.
Since all of these values are already available in scope at the point of emission, there is no additional storage cost, only a marginal increase in gas for the extra log topics and data.
Recommendation
Update the event signature and emission to include all deploy parameters:
event StablecoinDeployed( address indexed stablecoin, string name, string symbol, uint8 decimals, address indexed stablecoinAdmin, bytes32 indexed salt); emit StablecoinDeployed(stablecoin, name, symbol, decimals, stablecoinAdmin, salt);Documented immutability guarantee of BEACON does not hold
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
The NatSpec on StablecoinFactory states:
/// @dev The beacon is fixed at construction time as an immutable and cannot be changed.However, StablecoinFactory inherits UUPSUpgradeable, and Solidity immutable variables are embedded in the implementation contract's bytecode, not in proxy storage.
When
DEFAULT_ADMIN_ROLEperforms a UUPS upgrade, the proxy delegates to a new implementation whose constructor may have been called with a differentBEACONaddress. After the upgrade,factory.BEACON()returns the new implementation's value, effectively changing what the documentation claims "cannot be changed." This creates a discrepancy between the documented security property and the actual behavior of the system.Recommendation
Update the NatSpec to accurately reflect that BEACON is immutable per-implementation but can change across UUPS upgrades:
- /// @dev The beacon is fixed at construction time as an immutable and cannot be changed. + /// @dev The beacon is set at construction time as an immutable. Note that upgrading the+ /// factory implementation via UUPS will adopt the new implementation's BEACON value.Function supportsInterface() does not declare all supported interfaces
State
Severity
- Severity: Informational
Submitted by
Sujith S
Description
Stablecoin inherits
AccessControlDefaultAdminRulesUpgradeable, which implements ERC-165'ssupportsInterface()function. However, the contract does not override supportsInterface to declare support for the token-related interfaces it actually implements: ERC-20, ERC-2612, and ERC-3009.Any on-chain or off-chain system that calls supportsInterface to detect token capabilities such as wallets, aggregators, bridge contracts, or protocol routers will receive false for all three interfaces, despite the contract fully implementing them. Only AccessControl and ERC-165 itself will return true.
Recommendation
Override supportsInterface in Stablecoin to advertise the implemented token interfaces:
function supportsInterface(bytes4 interfaceId) public view override returns (bool) { return interfaceId == type(IERC20).interfaceId || interfaceId == type(IERC20Permit).interfaceId || interfaceId == type(IERC3009).interfaceId || super.supportsInterface(interfaceId); }Coinbase: Added the interfaces support checks for OZ contracts in 40b88d3. We don't have interface for ERC3009 so skipped that.
Cantina: Verified fix.