sherlock-audit / 2024-05-midas-judging

13 stars 6 forks source link

fandonov - The user can call the redeem function with any token he wants which can be bad for the protocol in some cases #74

Closed sherlock-admin4 closed 6 months ago

sherlock-admin4 commented 6 months ago

fandonov

medium

The user can call the redeem function with any token he wants which can be bad for the protocol in some cases

Summary

The user can call the redeem function with any token he wants.

Vulnerability Detail

The user can call the redeem function in the redemption vault and he can specify whatever token he likes. But as it is said in the documentation this shouldn't be the case because the redemption vault has its own list of supported USD tokens like the other vaults. All vaults do have it own lists of supported USD tokens. These are the exact words of how it is explained.

function redeem(address tokenOut, uint256 amountTBillIn)
        external
        onlyGreenlisted(msg.sender)
        whenNotPaused
    {
        require(amountTBillIn > 0, "RV: 0 amount");

        address user = msg.sender;

        lastRequestId.increment();
        uint256 requestId = lastRequestId.current();

        _requireTokenExists(tokenOut);
        _tokenTransferFromUser(address(mTBILL), amountTBillIn);

        emit Redeem(requestId, user, tokenOut, amountTBillIn);
    }

This is the redeem function which calls the _requireTokenExists function. Which is represented in the next way:

 function _requireTokenExists(address token) internal view override {
        if (token == MANUAL_FULLFILMENT_TOKEN) return;
        super._requireTokenExists(token);
    }

This only checks if the token == MANUAL_FULLFILMENT_TOKEN which is the address that represents off-chain USD bank transfer. The user can call the redeem function with a token that depreciates in price very often and get some kind of USD stablecoin instead, this way money can be lost. Yes, the admin can decide to cancel the deposit request but it is still likely that this can happen sometimes.

Impact

If the scenario explained above happens money can be lost.

Code Snippet

https://github.com/sherlock-audit/2024-05-midas/blob/a4a3cc23bb891913ce44665a4cdea9f5c1190f6c/midas-contracts/contracts/RedemptionVault.sol#L61-L77

https://github.com/sherlock-audit/2024-05-midas/blob/a4a3cc23bb891913ce44665a4cdea9f5c1190f6c/midas-contracts/contracts/RedemptionVault.sol#L90C4-L93C6

Tool used

Manual Review

Recommendation

Make a list of tokens that can be used and put checks in the function which check if the user who is depositing the tokens is depositing a token from the list.