Skip to content

Commit e59b05f

Browse files
committed
fix and document reentrancy risk
1 parent b9709dc commit e59b05f

File tree

1 file changed

+12
-2
lines changed

1 file changed

+12
-2
lines changed

contracts/token/ERC20/extensions/ERC4626.sol

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,11 @@ abstract contract ERC4626 is ERC20, IERC4626 {
9393

9494
address caller = _msgSender();
9595
uint256 shares = previewDeposit(assets);
96-
_mint(receiver, shares);
96+
97+
// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
98+
// the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
9799
SafeERC20.safeTransferFrom(_asset, caller, address(this), assets);
100+
_mint(receiver, shares);
98101

99102
emit Deposit(caller, receiver, assets, shares);
100103

@@ -107,8 +110,11 @@ abstract contract ERC4626 is ERC20, IERC4626 {
107110

108111
address caller = _msgSender();
109112
uint256 assets = previewMint(shares);
110-
_mint(receiver, shares);
113+
114+
// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
115+
// the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
111116
SafeERC20.safeTransferFrom(_asset, caller, address(this), assets);
117+
_mint(receiver, shares);
112118

113119
emit Deposit(caller, receiver, assets, shares);
114120

@@ -130,6 +136,8 @@ abstract contract ERC4626 is ERC20, IERC4626 {
130136
_spendAllowance(owner, caller, shares);
131137
}
132138

139+
// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
140+
// the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
133141
_burn(owner, shares);
134142
SafeERC20.safeTransfer(_asset, receiver, assets);
135143

@@ -153,6 +161,8 @@ abstract contract ERC4626 is ERC20, IERC4626 {
153161
_spendAllowance(owner, caller, shares);
154162
}
155163

164+
// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
165+
// the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
156166
_burn(owner, shares);
157167
SafeERC20.safeTransfer(_asset, receiver, assets);
158168

0 commit comments

Comments
 (0)