sherlock-audit / 2024-05-midas-judging

13 stars 6 forks source link

krot-0025 - Incorrect Conversion Method in `redeem` Function Leading to Potential Loss of Funds #180

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

krot-0025

high

Incorrect Conversion Method in redeem Function Leading to Potential Loss of Funds

Summary

The redeem function in the RedemptionVault contract uses an incorrect conversion method (convertFromBase18), which can lead to incorrect redemption amounts if the input amount is already in the token's native decimal format. This can result in users receiving less than the intended amount of the redemption token, causing financial discrepancies and potential loss of funds.

Vulnerability Detail

The redeem function in the RedemptionVault contract uses an incorrect conversion method (convertFromBase18), which can lead to incorrect redemption amounts if the input amount is already in the token's native decimal format. This can result in users receiving less than the intended amount of the redemption token, causing financial discrepancies and potential loss of funds.

Vulnerability Detail

The redeem function is designed to allow users to redeem their mTBILL tokens for another token (e.g., USDC). The amount of mTBILL tokens (amountTBillIn) is expected to be in base 18 format. However, the function uses the convertFromBase18 method, which converts the amount from 18 decimals to the target token's decimals (e.g., 6 for USDC). This conversion is incorrect because the amountTBillIn should already be in the correct format, leading to an additional and erroneous scaling down.

Detailed Example

Consider the scenario where a user wants to redeem 100 mTBILL (an 18 decimal token) for USDC (a 6 decimal token):

  1. User Input: The user calls redeem(tokenOut, 100 * 10**18), where amountTBillIn is 100 * 10**18.
  2. Expected Conversion:

    • The function should transfer 100 * 10**18 mTBILL, but due to the conversion method used, it performs:
    amountInTokenDecimals = (100 * 10**18).convertFromBase18(6);
    • This scales down the amount to 100 * 10**6, which is significantly less.
  3. Actual Transfer:
    • The contract attempts to transfer 100 * 10**6 (100 USDC) instead of 100 * 10**18 (correct mTBILL amount).
  4. Result:
    • The user receives only 100 USDC worth of mTBILL, whereas they should have received the equivalent of 100 * 10**18.

Numeric Difference

Calculation Details:

Impact

The incorrect conversion can cause users to receive significantly less than the intended amount of the redemption token. This discrepancy can lead to financial losses for users and undermine the integrity of the redemption process within the contract.

Potential Scenarios:

Code Snippet

Changes to be made in the code ::

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

Tool used

Manual Review

Recommendation

Ensure the amountTBillIn is correctly handled without unnecessary conversions. I am not sure about this recommendation but this may help saving the losses.

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);
    // No conversion needed if amountTBillIn is already in 18 decimals
-   _tokenTransferFromUser(address(mTBILL), amountTBillIn);
     // Transfer mTBILL tokens directly to the user's address
+    IERC20(address(mTBILL)).safeTransferFrom(user, tokenOut, amountTBillIn);

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