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

Fee on transfer tokens are not supported in ERC20Minter deposit/redeem/ withdraw functions #23

Open
hats-bug-reporter bot opened this issue Sep 2, 2024 · 1 comment
Labels
Invalid - Lead auditor invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: --
Twitter username: --
Submission hash (on-chain): 0xa55345ec35527c62257af4e8bccb5d41c5fe773e19ad85ec6acf763936ddcab4
Severity: medium

Description:
Description
Minter.sl provides Native ROSE token as well as ERC20 tokens as base tokens for liquid staking. The users can call deposit functions of NativeMinter.sol to deposit native ROSE token to get the exact staking tokens. Similarly, if the user wants to deposit ERC20 token like USDT/USDC, etc then they can call deposit function of ERC20Minter.sol which is implemented as below:

    function deposit(uint256 amount, address receiver) public virtual nonReentrant {
        require(amount > 0, "ZeroDeposit");
        uint256 mintAmount = previewDeposit(amount);
        require(mintAmount > 0, "ZeroMintAmount");
@>        baseToken.safeTransferFrom(address(msg.sender), address(this), amount);
        stakingToken.mint(receiver, mintAmount);
        emit Deposit(address(msg.sender), receiver, amount);
    }

For owner withdrawal of ERC20 base token, withdraw() function is implemented as:

    function withdraw(address receiver) public virtual onlyOwner {
        uint256 availableBalance = baseToken.balanceOf(address(this));
        require(availableBalance > 0, "ZeroWithdraw");
        baseToken.safeTransfer(receiver, availableBalance);
        emit Withdraw(address(msg.sender), receiver, availableBalance);
    }

For redeem of staking tokens and to get back the base token, redeem is implemeted as:

    function redeem(uint256 amount, address receiver) public virtual nonReentrant {
        require(amount > 0, "ZeroRedeem");
        uint256 redeemAmount = previewRedeem(amount);
        require(redeemAmount > 0, "ZeroRedeemAmount");
        stakingToken.safeTransferFrom(address(msg.sender), address(this), amount);
        stakingToken.burn(amount);
        baseToken.safeTransfer(receiver, redeemAmount);
        emit Redeem(address(msg.sender), receiver, amount);
    }

For owner withdrawal and user withdrawal, its implemented as:

    function withdraw(address receiver) public virtual onlyOwner override {
        uint256 balance = balanceAvailable();
        require(balance > 0, "BalanceNotEnough");
        baseToken.safeTransfer(receiver, balance);
        emit Withdraw(address(msg.sender), receiver, balance);
    }
    
    function claimWithdrawal(uint256 withdrawalId, address receiver) public virtual nonReentrant {
        require(ownerOf(withdrawalId) == msg.sender, "NotOwner");
        WithdrawalRequest storage request = _withdrawalRequests[withdrawalId];
        require(request.processed, "NotProcessedYet");
        require(!request.claimed, "AlreadyClaimed");
        _burn(withdrawalId);
        request.claimed = true;
        totalUnclaimedWithdrawals = totalUnclaimedWithdrawals-request.amount;
        baseToken.safeTransfer(receiver, request.amount);
        emit ClaimWithdrawal(address(msg.sender), receiver, request.amount, withdrawalId);
    }

The issue here is that, the base token can be any token including USDT i.e fee on transfer tokens.

Some tokens have a fee on transfer, for example USDT. Usually such fee is not enabled but could be re-enabled at any time. With this fee enabled the functions interacting with Fee on transfer tokens would receive slightly less tokens than the amounts requested.

Some ERC20 tokens(e.g. STA,PAXG, USDC,USDT) allow for charging a fee any time transfer()or transferFrom() is called.

In above ALL highlighted functions, Tokens with a fee on transfer is not supported which can cause accounting errors. The end user while transferring and claiming his base tokens can receive less tokens than expected.

Recommendations
Check the balanceOf() tokens before and after a safeTransfer() or safeTransferFrom() in case of base token. Use the difference as the amount of tokens sent/received.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 2, 2024
@ilzheev
Copy link

ilzheev commented Sep 3, 2024

Minter contracts are supposed to work only with protocol tokens, that can be staked (either gas tokens – ROSE, ZETA, SEI on corresponding chains or ERC20 versions on sidechains, e.g. ERC20 WROSE on BNB Chain, ERC20 ZETA on Ethereum, etc.).

Stablecoins with possible fee on transfer are not supposed to be used as base tokens in Minter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid - Lead auditor invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants