Skip to content

Commit

Permalink
fix and document reentrancy risk
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Mar 10, 2022
1 parent b9709dc commit fac4303
Showing 1 changed file with 12 additions and 2 deletions.
14 changes: 12 additions & 2 deletions contracts/token/ERC20/extensions/ERC4626.sol
Expand Up @@ -93,8 +93,11 @@ abstract contract ERC4626 is ERC20, IERC4626 {

address caller = _msgSender();
uint256 shares = previewDeposit(assets);
_mint(receiver, shares);

// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
// the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
SafeERC20.safeTransferFrom(_asset, caller, address(this), assets);
_mint(receiver, shares);

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

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

address caller = _msgSender();
uint256 assets = previewMint(shares);
_mint(receiver, shares);

// if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
// the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
SafeERC20.safeTransferFrom(_asset, caller, address(this), assets);
_mint(receiver, shares);

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

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

// if _asset is ERC777, transfer can call reenter AFTER the transfer happens through
// the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
_burn(owner, shares);
SafeERC20.safeTransfer(_asset, receiver, assets);

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

// if _asset is ERC777, transfer can call reenter AFTER the transfer happens through
// the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
_burn(owner, shares);
SafeERC20.safeTransfer(_asset, receiver, assets);

Expand Down

0 comments on commit fac4303

Please sign in to comment.