diff --git a/.changeset/thin-eels-cross.md b/.changeset/thin-eels-cross.md new file mode 100644 index 00000000000..7993d7d64df --- /dev/null +++ b/.changeset/thin-eels-cross.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`ERC4626`: Use the `asset` getter in `totalAssets`, `_deposit` and `_withdraw`. diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index ec9a255076c..338b71d62c5 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -114,7 +114,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** @dev See {IERC4626-totalAssets}. */ function totalAssets() public view virtual returns (uint256) { - return _asset.balanceOf(address(this)); + return IERC20(asset()).balanceOf(address(this)); } /** @dev See {IERC4626-convertToShares}. */ @@ -237,14 +237,14 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Deposit/mint common workflow. */ function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal virtual { - // If _asset is ERC-777, `transferFrom` can trigger a reentrancy BEFORE the transfer happens through the + // If asset() is ERC-777, `transferFrom` can trigger a reentrancy BEFORE the transfer happens through the // `tokensToSend` hook. On the other hand, the `tokenReceived` hook, that is triggered after the transfer, // calls the vault, which is assumed not malicious. // // Conclusion: we need to do the transfer before we mint so that any reentrancy would happen before the // assets are transferred and before the shares are minted, which is a valid state. // slither-disable-next-line reentrancy-no-eth - SafeERC20.safeTransferFrom(_asset, caller, address(this), assets); + SafeERC20.safeTransferFrom(IERC20(asset()), caller, address(this), assets); _mint(receiver, shares); emit Deposit(caller, receiver, assets, shares); @@ -264,14 +264,14 @@ abstract contract ERC4626 is ERC20, IERC4626 { _spendAllowance(owner, caller, shares); } - // If _asset is ERC-777, `transfer` can trigger a reentrancy AFTER the transfer happens through the + // If asset() is ERC-777, `transfer` can trigger a reentrancy AFTER the transfer happens through the // `tokensReceived` hook. On the other hand, the `tokensToSend` hook, that is triggered before the transfer, // calls the vault, which is assumed not malicious. // // Conclusion: we need to do the transfer after the burn so that any reentrancy would happen after the // shares are burned and after the assets are transferred, which is a valid state. _burn(owner, shares); - SafeERC20.safeTransfer(_asset, receiver, assets); + SafeERC20.safeTransfer(IERC20(asset()), receiver, assets); emit Withdraw(caller, receiver, owner, assets, shares); }