code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/StableVault.sol#L56 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L647

Vulnerability details

Impact

Some tokens may not conform with IERC20Permit. Case in point, DAI stablecoin which uses a permit() that is different than the reference permit.

IERC20Permit:

    function permit(
        address owner,
        address spender,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external;

DAI's permit:

    function permit(
        address holder,
        address spender,
        uint256 nonce,
        uint256 expiry,
        bool allowed,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external;

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

Either check if the token being permitted is DAI or not, and use DAI's permit if it is. Alternatively, Uniswap's permit2.

GalloDaSballo commented 1 year ago

Looks valid, but no impact, meaning I will most likely downgrade due to a lack of showing how the finding can cause a loss beside it not working with DAI