Skip to content

Commit

Permalink
Document risk of SafeERC20 and ERC-7674 (#5262)
Browse files Browse the repository at this point in the history
Signed-off-by: Hadrien Croubois <[email protected]>
  • Loading branch information
cairoeth authored and Amxx committed Oct 17, 2024
1 parent 81c7206 commit 4ddb8d8
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-tables-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`SafeERC20`: Document risks of `safeIncreaseAllowance` and `safeDecreaseAllowance` when associated with ERC-7674.
14 changes: 14 additions & 0 deletions contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ library SafeERC20 {
/**
* @dev Increase the calling contract's allowance toward `spender` by `value`. If `token` returns no value,
* non-reverting calls are assumed to be successful.
*
* IMPORTANT: If the token implements ERC-7674 (ERC-20 with temporary allowance), and if the "client"
* smart contract uses ERC-7674 to set temporary allowances, then the "client" smart contract should avoid using
* this function. Performing a {safeIncreaseAllowance} or {safeDecreaseAllowance} operation on a token contract
* that has a non-zero temporary allowance (for that particular owner-spender) will result in unexpected behavior.
*/
function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal {
uint256 oldAllowance = token.allowance(address(this), spender);
Expand All @@ -55,6 +60,11 @@ library SafeERC20 {
/**
* @dev Decrease the calling contract's allowance toward `spender` by `requestedDecrease`. If `token` returns no
* value, non-reverting calls are assumed to be successful.
*
* IMPORTANT: If the token implements ERC-7674 (ERC-20 with temporary allowance), and if the "client"
* smart contract uses ERC-7674 to set temporary allowances, then the "client" smart contract should avoid using
* this function. Performing a {safeIncreaseAllowance} or {safeDecreaseAllowance} operation on a token contract
* that has a non-zero temporary allowance (for that particular owner-spender) will result in unexpected behavior.
*/
function safeDecreaseAllowance(IERC20 token, address spender, uint256 requestedDecrease) internal {
unchecked {
Expand All @@ -70,6 +80,10 @@ library SafeERC20 {
* @dev Set the calling contract's allowance toward `spender` to `value`. If `token` returns no value,
* non-reverting calls are assumed to be successful. Meant to be used with tokens that require the approval
* to be set to zero before setting it to a non-zero value, such as USDT.
*
* NOTE: If the token implements ERC-7674, this function will not modify any temporary allowance. This function
* only sets the "standard" allowance. Any temporary allowance will remain active, in addition to the value being
* set here.
*/
function forceApprove(IERC20 token, address spender, uint256 value) internal {
bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value));
Expand Down

0 comments on commit 4ddb8d8

Please sign in to comment.