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

depositWithSignature() won't work and always revert due to missing permit function in deposited asset #62

Open
hats-bug-reporter bot opened this issue Sep 9, 2024 · 0 comments
Labels
bug Something isn't working OOS - fixed

Comments

@hats-bug-reporter
Copy link

Github username: @0xRizwan
Twitter username: 0xRizwann
Submission hash (on-chain): 0x65db4f9773c0e4f7ecce60decd9cb1711a85e625ffec2b6bf429a25b1954497d
Severity: medium

Description:

Title

depositWithSignature() won't work and always revert due to missing permit function in deposited asset.

Severity

Medium

Affected contracts

wrstMTRG.sol, wstARB.sol, wstDOJ.sol, wstMANTA.sol, wstMETIS.sol, wstROSE.sol, wstVLX.sol, wstZETA.sol and wstToken.sol

Vulnerability Detail

wstToken.sol is the referred contract from which the above contracts are deployed. wstToken is ERC4626 compatible. For example, in case of wstROSE.sol, it allows to deposit stROSE token and mints wstROSE tokens. This issue is with depositWithSignature() function of wstToken.sol:

    function depositWithSignature(
        uint256 assets,
        address receiver,
        uint256 deadline,
        bool approveMax,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external nonReentrant returns (uint256 shares) {
        uint256 amount = approveMax ? type(uint256).max : assets;
        asset.permit(msg.sender, address(this), amount, deadline, v, r, s);
        return (deposit(assets, receiver));
    }

depositWithSignature() is used to deposit the assets via permit and later deposits in single transaction. depositWithSignature() will always revert and won't work as expected as the asset being permitted to address(this) i.e wstToken.sol contract does not have permit() function in its implementation. This can be checked in stToken.sol here

Therefore, assets like rstMTRG.sol, stARB.sol, stDOJ.sol, stMANTA.sol, stMETIS.sol, stROSE.sol, stVLX.sol, stZETA.sol tokens does not have permit() function to work correctly in depositWithSignature() function for depositing of stToken in vaults.

Impact

Assets can not be deposited in wstToken contracts due to missing permit() function in deposited assets as described above.

Recommendation to fix

Implement permit() function in stToken.sol and also make relevant changes in rstMTRG.sol, stARB.sol, stDOJ.sol, stMANTA.sol, stMETIS.sol, stROSE.sol, stVLX.sol, stZETA.sol contracts.

Consider below changes in stToken.sol:

  1. Add this openzeppelin's ERC20Permit.sol in stToken.sol.

  2. Inherit the added ERC20Permit.sol.

- contract stToken is ERC20, ERC20Burnable, Pausable, Ownable {
+ contract stToken is ERC20, ERC20Burnable, ERC20Permit, Pausable, Ownable {
    constructor(
        string memory name,
        string memory symbol,
        uint8 decimals
-     ) ERC20(name, symbol, decimals) {}
+    ) ERC20(name, symbol, decimals) ERC20Permit(name) {}

     . . . some code . . . 
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working OOS - fixed
Projects
None yet
Development

No branches or pull requests

1 participant