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

Call to permit() will kill previous allowances of the spender #418

Open
shealtielanz opened this issue Aug 14, 2024 · 1 comment
Open

Call to permit() will kill previous allowances of the spender #418

shealtielanz opened this issue Aug 14, 2024 · 1 comment

Comments

@shealtielanz
Copy link

summary

on the call to erc20#permit it sets the allowance of the spender to the value instead of simply adding to it, it makes sense to add to it supposing a spender already has previous allowance of the owner.
https://github.com/transmissions11/solmate/blob/bfc9c25865a274a7827fea5abf6e4fb64fc64e6c/src/tokens/ERC20.sol#L116C1-L160C6

mitigation

    function permit(
        address owner,
        address spender,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) public virtual {
        require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");
..SNIP..

            require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");

 --           allowance[recoveredAddress][spender] = value;
++           allowance[recoveredAddress][spender] += value;
        }

        emit Approval(owner, spender, value);
    }
@MathisGD
Copy link

This would break EIP-2612
image

Also you wouldn't be able to decrease the allowance using permit (not sure that there is a use-case for this, but flagging anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants