transmissions11 / solmate

Modern, opinionated, and gas optimized building blocks for smart contract development.
GNU Affero General Public License v3.0
3.93k stars 645 forks source link

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

Open shealtielanz opened 2 months ago

shealtielanz commented 2 months ago

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 commented 1 month ago

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).