Both CashKYCSender and CashKYCSenderReceiver variants of the CASH token validate that when a transfer is executed, the caller of the function must be KYC approved . This is done in the _beforeTokenTransfer function:
function _beforeTokenTransfer(
address from,
address to,
uint256 amount
) internal override {
super._beforeTokenTransfer(from, to, amount);
require(
_getKYCStatus(_msgSender()),
"CashKYCSender: must be KYC'd to initiate transfer"
);
if (from != address(0)) {
// Only check KYC if not minting
require(
_getKYCStatus(from),
"CashKYCSender: `from` address must be KYC'd to send tokens"
);
}
}
This hook is not only called as part of the transfer function, but is also triggered during a call to transferFrom.
The OpenZeppelin implementation of the ERC20 calls the _transfer internal function as part of the transferFrom implementation, which ends up calling the _beforeTokenTransfer hook.
Impact
The transferFrom function is used extensively by different DEFI protocols that integrate with ERC20 tokens.
The current restriction will prevent most protocols from integrating with the CASH token, even if the particular sender and/or receiver are KYC approved.
The current implementation will require that each of these protocols have an approved KYC status, which is a bit hard to imagine since these are decentralized smart contracts.
Recommendation
This may need further discussion. A first step would be to relax the KYC constraint imposed in the caller. This would allow DEFI contracts to operate using transferFrom on behalf of a KYC approved sender and/or receiver. Care must be taken if transferring tokens into, for example, a pool.
Lines of code
https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/token/CashKYCSender.sol#L63-L66 https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/token/CashKYCSenderReceiver.sol#L63-L66
Vulnerability details
Both
CashKYCSender
andCashKYCSenderReceiver
variants of the CASH token validate that when a transfer is executed, the caller of the function must be KYC approved . This is done in the_beforeTokenTransfer
function:https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/token/CashKYCSender.sol#L56-L75
This hook is not only called as part of the
transfer
function, but is also triggered during a call totransferFrom
.The OpenZeppelin implementation of the ERC20 calls the
_transfer
internal function as part of thetransferFrom
implementation, which ends up calling the_beforeTokenTransfer
hook.Impact
The
transferFrom
function is used extensively by different DEFI protocols that integrate with ERC20 tokens.The current restriction will prevent most protocols from integrating with the CASH token, even if the particular sender and/or receiver are KYC approved.
The current implementation will require that each of these protocols have an approved KYC status, which is a bit hard to imagine since these are decentralized smart contracts.
Recommendation
This may need further discussion. A first step would be to relax the KYC constraint imposed in the caller. This would allow DEFI contracts to operate using
transferFrom
on behalf of a KYC approved sender and/or receiver. Care must be taken if transferring tokens into, for example, a pool.