Deposit Will Not Work Correct

Summary

The DepositVault::deposit function exhibits incorrect behavior regardless of whether the user passes the USD amount scaled to 10^18 or not scaled at all.

Vulnerability Detail

DepositVault::deposit it's not working correct. Let's dive deep into it:

Incorrect Scaling of USD Amount

Currently, the DepositVault::deposit function expects the USD amount to be scaled off-chain to 18 decimals, as per the method documentation. However, due to a flaw in the implementation, the minAmountToDepositInUsd check will always return a value that is not scaled to the expected 18 decimals. This discrepancy arises from the division by 10 ** 18, leading to an inaccurate comparison. For instance, if minAmountToDepositInEuro is set to 100,000(This value was referred by the devs), the check will always evaluate to true, rendering the validation ineffective.

Handling of Unscaled Amounts

Additionally, the function encounters issues when the user provides the amount without scaling it to 18 decimals. In such cases, if the value surpasses DepositVault::minAmountToDepositInUsd, the function proceeds to update totalDeposited[user] with the unscaled amount. Subsequently, when attempting to send the amount from the user to the contract, the function incorrectly scales down the decimals from 18 to match the token's decimal (e.g., 8 decimals for USDC). However, as the provided value is not scaled to 18 decimals, the scaling down operation is likely to result in 0, leaving the contract in an inconsistent state.

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/DepositVault.sol#L91-L112

Impact

Ineffective Validation: The incorrect scaling of the USD amount undermines the effectiveness of the minAmountToDepositInUsd check, potentially allowing deposits that do not meet the specified criteria.
Contract Inconsistency: Mishandling unscaled amounts can lead to inconsistencies in the contract state, posing risks to its integrity and functionality.

Code Snippet

/**
* @inheritdoc IDepositVault
* @dev transfers `tokenIn` from `msg.sender`
* to `tokensReceiver`
* @param tokenIn address of token to deposit.
* @param amountUsdIn amount of token to deposit in 10**18 decimals.
*/
function deposit(address tokenIn, uint256 amountUsdIn) external onlyGreenlisted(msg.sender) whenNotPaused {

    address user = msg.sender;
    _requireTokenExists(tokenIn);
    lastRequestId.increment();

    uint256 requestId = lastRequestId.current();

    if (!isFreeFromMinDeposit[user]) {
        _validateAmountUsdIn(user, amountUsdIn);
    }

    require(amountUsdIn > 0, "DV: invalid amount");

    totalDeposited[user] += amountUsdIn;

    _tokenTransferFromUser(tokenIn, amountUsdIn);

    emit Deposit(requestId, user, tokenIn, amountUsdIn);
}

/**
* @notice minAmountToDepositInEuro converted to USD in base18
*/
function minAmountToDepositInUsd() public view returns (uint256) {
    return (minAmountToDepositInEuro * eurUsdDataFeed.getDataInBase18()) /
          10 ** 18;
}

Tool used

Manual Review

Recommendation

Correct Scaling: Revise the scaling mechanism used in the minAmountToDepositInUsd check to ensure accurate comparison of scaled USD amounts.
Standardized Decimal Handling: Implement standardized procedures for handling both scaled and unscaled amounts consistently, mitigating the risk of contract inconsistencies.
Thorough Testing: Conduct comprehensive testing to validate the behavior of the DepositVault::deposit function under various scenarios, ensuring its reliability and resilience against potential exploits.

Duplicate of #108

sherlock-audit / 2024-05-midas-judging

petarP1998 - Deposit Will Not Work Correct #173