Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document risk of SafeERC20 and ERC-7674 #5262

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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