-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Add EIP-5748: Approval Expiration for EIP-20 Tokens #5748
Changes from all commits
7781c2c
83890f9
fd971da
9127219
3ef9e71
0556b62
6b9c8f5
314bccc
7b6cb76
aed99f0
c5817f1
51d77b1
a7d068a
f756e39
b74515a
506bdff
93fa230
65d3750
e6dd69c
b615e30
470912b
b0183f0
d9ffc78
96dc083
8db8319
df609c6
9a368a2
77bc9b3
a11982f
86cc023
836dc7e
6fde6e8
1846db3
abfe631
84631f1
5459e2f
d098211
a1e8bb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
--- | ||
eip: 5748 | ||
title: Approval Expiration for EIP-20 Tokens | ||
description: Extending EIP-20 approvals to automatically expire after a duration | ||
author: Go+ Security (@GoPlusSecurity), Jeff Hall (@Mr-Lucky), Xavi (@XaaaaavitheFool), Turan Vural (@turanzv), Johans Ballestar (@ballestar), Eskil (@xyfidea) | ||
discussions-to: https://ethereum-magicians.org/t/eip-5748-automatic-token-approval-expiration/11185 | ||
status: Draft | ||
type: Standards Track | ||
category: Interface | ||
created: 2022-10-02 | ||
requires: 20 | ||
--- | ||
|
||
## Abstract | ||
|
||
This standard adds an expiration time to the `_approve()` function of token contracts to automatically recall approvals within a certain time period. This functionality as a standard will prevent vulnerabilty exploits due to outstanding token approvals. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect |
||
|
||
## Motivation | ||
|
||
As of the writing of this standard, the approval attack is the most common attack method in the industry with nearly all phishing attacks and asset theft related to it. Although good usage habits and basic knowledge of on-chain security can help users avoid a multitiude of exploits such as phishing, scams, DNS hijacking, JS injection, and other common approval attack methods, the spender's own security (or lack thereof) still needs to be fortified by strong code. | ||
|
||
In the Transit Swap attack of October 1st, 2022, hackers used a vulnerability in the Transit Swap contract to steal $20 million in assets. Due in part to professional security audits and the strong brand recognition of Transit Swap, many users were given a false sense of security and trusted the platform's service. Transit Swaps's contract had been running smoothly for over a year and had accumulated a large amount of user allownces, making the contract a high-reward target for malicious actors. | ||
|
||
There have been many similar incidents to the Transit Swap attack. Security audits cannot gurantee that a contract is 100% free from vulnerabilities. Some vulnerabilities are intentionally left by malicious developers themselves. In line with good trust practices and general security hygeine, unconditional trust is risky, and modifying approvals into limited trusts with automatic recovery will be a key tool to natively prevent attacks. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This motivation is good background (though it might be polite to avoid mentioning a specific company/product), but it doesn't actually convince me that an expiring approval solves these problems. Maybe add a sentence along the lines of: If these lingering approvals had instead expired, the number of users affected by this attack would have been much smaller. |
||
## Specification | ||
|
||
```solidity | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move to Reference Implementation if you didn't intend to specify a specific behavior but only the interface. |
||
pragma solidity ^0.8.0; | ||
|
||
/// @title Approval Expiration for EIP-20 Tokens | ||
|
||
interface IERC20ApprovalExpire { | ||
/** | ||
* @dev Emitted when the allowance of a `spender` for an `owner` is set by a call to {approve} or {transferFrom}. | ||
*/ | ||
event AllowanceExpirationUpdated(address indexed _owner, address indexed _spender, uint256 _value, uint256 _expireAt); | ||
|
||
|
||
/** | ||
* @dev returns the default expiration time for approval | ||
*/ | ||
function defaultExpiration() external pure returns (uint256); | ||
|
||
/** | ||
* @dev Return the expire timestamp of the Approval which the address owner approve to the spender. _expiration[_owner][_spender] | ||
*/ | ||
function allowanceExpiration(address _owner, address _spender) external view returns(uint256); | ||
|
||
/** | ||
* @dev | ||
* | ||
* Returns a boolean value indicating whether the operation succeeded. | ||
* | ||
* the origin ERC20 approve, increaseAllowance, decreaseAllowance functions need to use DEFAULT_EXPIRATION | ||
* as the params ‘period’ when call this internal _approve function. | ||
* In this function, set _expiration[owner][spender] = block.timestamp + period, | ||
* and emit AllowanceExpirationUpdated event. | ||
* | ||
* Emits an {AllowanceExpirationUpdated} event. | ||
* Emits an {Approval} event. | ||
* | ||
* In the success case | ||
*/ | ||
function approve(address _spender, uint256 _value, uint256 _period) external returns (bool); | ||
|
||
/** | ||
* @dev Add the param 'period' for ERC20.increaseAllowance | ||
* Default increaseAllowance is overridden if the period is specified | ||
*/ | ||
function increaseAllowance(address _spender, uint256 _addedValue, uint256 _period) external returns (bool); | ||
|
||
/** | ||
* @dev Add the param 'period' for ERC20.decreaseAllowance | ||
* Default decreaseAllowance is overridden if the period is specified | ||
*/ | ||
function decreaseAllowance(address spender, uint256 subtractedValue, uint256 period) external returns (bool); | ||
} | ||
|
||
``` | ||
|
||
A time period `_expiration` is mapped on top of the original `allowance` to specify an approval time limit. | ||
|
||
The `_approve()` method adds uint256 `period` to the entry, which is stored in `_expiration`. | ||
|
||
Contracts implementing this standard MUST expose a `DEFAULT_EXPIRATION` constant of type `uint256`. This constant stores the expiration period to use for `approve` calls that do not include a `period` argument. | ||
|
||
`approve` functions without a `period` argument MUST use the expiry period exposed in the `DEFAULT_EXPIRATION` constant. | ||
|
||
In order not to interfere with the existing event statistics and to allow other developers to count the validity of allowance, we add a new event `AllowanceExpireTimeUpdated()`, which will be triggered in the `_approve()` method. | ||
|
||
A separate approval method with the header `approve(address spender, uint256 amount, uint256 period)` modifies the `_expiration` value for existing approvals. | ||
|
||
Transactions using the `transferFrom()` method will be checked against `_expiration` through `_spendAllowance()`. Transactions that take place after the alloted time period will fail. | ||
|
||
The `allowance()`, `increaseAllowance()`, and `decreaseAllowance()` methods have been modified to accommodate the new features. `allowanceExpiration()` has been added to query the live period of the contract. | ||
|
||
The above modifications adds a validity period to [EIP-20](./eip-20.md) token approvals. Users can freely update their allowance validity at any time by calling `approve(address spender, uint256 amount, uint256 period)`. | ||
|
||
## Rationale | ||
|
||
`_expiration` is implemented as a mapping to improve the compatibility of the code. | ||
|
||
A separate approval method with the header `approve(address spender, uint256 amount, uint256 period)` has been introduced to allow users to customize the `_expiration` value of the approval. | ||
|
||
The internal method `_spendAllowance()` is introduced for code cleanliness with the function of checking the allowance amount and expiration. | ||
|
||
The `allowance()` method has been modified to accommodate the functionality described in this standard. `allowanceExpiration()` has been added query the allowance expiration date. | ||
|
||
## Backwards Compatibility | ||
|
||
This standard is compatible with the [EIP-20](./eip-20.md), [EIP-721](./eip-721.md) and [EIP-1155](./eip-1155.md) standards. Tokens issued in the form of proxy contracts can be updated to comply with this standard. | ||
|
||
So that existing event statistics are not perturbed by the implementation of this standard, `_approve()` is triggered by the new event `AllowanceExpireTimeUpdated()`. | ||
|
||
This standard is absolutely compatible with the traditional [EIP-20](./eip-20.md) standard. `DEFAULT_EXPIRATION` is added so that the original `approve(address spender, uint256 amount)` method header can remain for backwards compatibility and ease of programming. `DEFAULT_EXPIRATION` can be set to a constant or variable according to the developer's needs. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we know this is 'absolutely compatible' ? |
||
|
||
## Reference Implementation | ||
|
||
Implementation can be referenced in `../assets/eip-5748/`. | ||
|
||
## Security Considerations | ||
|
||
When upgrading a standard [EIP-20](./eip-20.md)-based proxy contract to this standard, attention should be paid to the location of assets. | ||
|
||
## Copyright | ||
|
||
Copyright and related rights waived via [CC0](../LICENSE.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This proposal doesn't add an expiration time, but rather a validity period after the transaction is included on-chain. I know it's a pedantic difference, but it's important from a security point of view.