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

Permit does not allow partial allowances #141

Closed
smoelius opened this issue Mar 27, 2020 · 1 comment
Closed

Permit does not allow partial allowances #141

smoelius opened this issue Mar 27, 2020 · 1 comment
Labels
wontfix This will not be worked on

Comments

@smoelius
Copy link

Permit does not allow partial allowances

Severity: Informational
Difficulty: High

Description

permit allows one to approve only zero or everything:

uint wad = allowed ? uint(- 1) : 0;
_approve(holder, spender, wad);

This schema creates an incentive to behave maliciously for the spender. Having a decreasePermit/increasePermit would be safer, in particular for holders with large token balances.

Exploit Summary

Alice signs a permit for Bob to spend NGNT tokens on her behalf. Since Alice trusts Bob, she does not find this concerning. Eve holds Bob at gunpoint and forces him to give up his private keys. Eve uses Bob's private keys and the signed permit to steal all of Alice's NGNT tokens.

Recommendation

Implement decreasePermit and increasePermit functions that decrease and increase a spender's allowance (respectively), similar to OpenZeppelin's decreaseAllowance and increaseAllowance functions.

@prekucki prekucki added the rqc label Mar 31, 2020
@shadeofblue
Copy link
Collaborator

by a design choice, permit will never be used that way.

We'll clearly recommend against issuing permits to individual private keys and thus permits will only be given to contracts. There will be a very select, presumably also audited set of contracts that we will ever recommend issuing permit for.

@shadeofblue shadeofblue added wontfix This will not be worked on and removed rqc labels Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants