Omniscia Nexera Audit

ERC20Transfers Manual Review Findings

ERC20Transfers Manual Review Findings

ERC-01M: Improper Allowance Bypass

Description:

The ERC20Transfers::_transferFrom function implementation is meant to bypass allowance checks for usage in recovery situations, however, it fails to invoke the ERC20Transfers::_beforeTokenTransfer function hook as well resulting in significant issues within downstream contracts.

Impact:

The ERC20Transfers::_transferFrom function implementation remains unutilized within the codebase, rendering this particular issue to be of informational severity.

Example:

contracts/dataManagers/ERC20/extensions/ERC20Transfers.sol
29/**
30 * @notice Transfers `amount` tokens from `from` to `to`
31 * @param from Address to transfer tokens from
32 * @param to Address to transfer tokens to
33 * @param amount Amount of tokens to transfer
34 * @return True if the transfer was successful, reverts otherwise
35 * @dev The caller must have allowance for `from`'s tokens of at least `amount`
36 */
37function transferFrom(address from, address to, uint256 amount) external virtual override returns (bool) {
38 _spendAllowance(from, _msgSender(), amount);
39 _beforeTokenTransfer(from, to, amount);
40
41 return _transferFrom(from, to, amount);
42}
43
44function _transferFrom(address from, address to, uint256 amount) internal virtual returns (bool) {
45 if (from == address(0)) {
46 revert ERC20InvalidSender(address(0));
47 }
48 if (to == address(0)) {
49 revert ERC20InvalidReceiver(address(0));
50 }
51
52 _writeTransfer(from, to, amount);
53
54 emit Transfer(from, to, amount);
55 return true;
56}

Recommendation:

We advise the ERC20Transfers::_beforeTokenTransfer function to be invoked regardless, ensuring derivative contracts that are recoverable properly accommodate for recovery scenarios within it.

Alleviation:

The ERC20Transfers::_beforeTokenTransfer function has been relocated to the ERC20Transfers::_transferFrom implementation, ensuring it is consistently invoked as expected.