6.1 Blacklisted spenders can still execute transfers via transferFrom(...)
The StandardGoldCoin contract implements a blacklist mechanism designed to freeze malicious accounts and prevent them from participating in transactions. This restriction is enforced by overriding the _update(...) function to check both the sender ( from) and the recipient ( to).
However, the contract inherits the standard ERC20 transferFrom(...) logic without overriding it or _spendAllowance(...) to validate the status of the spender.
In a transferFrom(...) operation, the msg.sender is the spender. Since _update(...) only verifies the token owner and the recipient, a blacklisted address acting as a spender can still move funds on behalf of other users. This bypasses the intent of the blacklist, which is to isolate the malicious account from the protocol.
// @audit The _spendAllowance function doesn't check if the spender is blacklisted or not
function _update(address from, address to, uint256 value) internal override whenNotPaused {
// Enforce blacklist (skip for mint/burn)
if (from != address(0)) require(!_blacklisted[from], "SGC: sender blacklisted");
if (to != address(0)) require(!_blacklisted[to], "SGC: recipient blacklisted");
// ...
}
Recommendation
Consider overriding the _spendAllowance(...) function to explicitly require that msg.sender (the spender) is not blacklisted.
6.2 The EOA check in initialize(...) can be bypassed by contracts during construction
The initialize(...) function attempts to distinguish whether the provided admin address is an Externally Owned Account (EOA) or a smart contract to correctly assign the feeCollector. The protocol explicitly intends for the feeCollector to be address(0) if the admin is a contract to avoid assigning a contract as a fee collector.
However, the logic relies on admin.code.length == 0 to verify EOA status. In Solidity, a smart contract returns a code size of 0 while it is being executed inside its own constructor.
This creates an edge case where a smart contract could call initialize(...) during its construction phase. The check would falsely identify it as an EOA, resulting in the contract address being assigned as the feeCollector, contrary to the protocol's intended design constraints.
function initialize(address admin) public initializer {
// ...
// @audit-issue Returns 0 if admin is a contract currently being constructed
if (admin.code.length == 0) {
feeCollector = admin;
emit FeeCollectorChanged(address(0), admin);
} else {
feeCollector = address(0);
emit FeeCollectorChanged(address(0), address(0));
}
}
Recommendation
Consider documenting this behavior to ensure operational teams are aware of it.
6.3 supportsInterface returns false for ERC20, Metadata, and Permit interfaces
The contract overrides supportsInterface to ensure compatibility with AccessControlUpgradeable. However, the implementation simply delegates to super.supportsInterface.
Since OpenZeppelin's ERC20Upgradeable and ERC20PermitUpgradeable do not implement ERC165 by default, the super call chain does not account for them. Consequently, the contract reports that it does not support the following standard interfaces, despite fully implementing them IERC20, IERC20Metadata, and IERC20Permit.
This creates a discrepancy where the contract functions as an ERC20/Permit token but denies supporting these interfaces.
// ...
function supportsInterface(bytes4 interfaceId) public view override(AccessControlUpgradeable) returns (bool) {
// @audit Returns false for IERC20, IERC20Metadata, and IERC20Permit
return super.supportsInterface(interfaceId);
}
Recommendation
Consider manually checking for these interface IDs to provide accurate contract information.
Update from Nethermind Security
You don't need to manually calculate interface ID. You can do something like:
return interfaceId == type(IERC20).interfaceId ||
interfaceId == type(IERC20Metadata).interfaceId ||
interfaceId == type(IERC20Permit).interfaceId ||
super.supportsInterface(interfaceId);
6.4 The implementation contract does not call _disableInitializers(...) in the constructor
The StandardGoldCoin contract is designed as an upgradeable contract that inherits from Initializable and UUPSUpgradeable. It relies on the initialize(...) function to set up the initial state, such as granting the DEFAULT_ADMIN_ROLE and other roles to the admin address.
However, the contract does not implement a constructor that calls _disableInitializers().
This means that the logic contract (the implementation) can be initialized by any third party after deployment. According to OpenZeppelin's best practices for upgradeable contracts, the implementation contract should be "locked" in the constructor to prevent it from being used or initialized directly. Leaving it open can allow an attacker to claim ownership of the implementation contract.
contract StandardGoldCoin is
Initializable,
ERC20Upgradeable,
ERC20PermitUpgradeable,
AccessControlUpgradeable,
PausableUpgradeable,
UUPSUpgradeable
{
// ...
// @audit-issue The contract lacks a constructor calling _disableInitializers().
//..
}
Recommendation
Consider adding a constructor to the contract that calls _disableInitializers(). This ensures that the implementation contract is locked upon deployment and cannot be initialized by an attacker later on.
