hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

Risk of losing funds because of non-existent`receiver` address #58

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

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

Description: Description

users give the receiver address for redeemAtom() and redeemTriple() to get funds for the provided address but the issue is according to the Solidity docs The low-level functions call, delegatecall, and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed. so users can mistakenly provide a wrong address that is non-existent and because a low-level call returns true the bool success check wouldn't be effective and the user who provided the wrong address ends up losing funds.

Impact

loss of funds for users.

Code Snippet

function redeemAtom(uint256 shares, address receiver, uint256 id) external nonReentrant returns (uint256) {
        if (id == 0 || id > count) {
            revert Errors.MultiVault_VaultDoesNotExist();
        }

        if (isTripleId(id)) {
            revert Errors.MultiVault_VaultNotAtom();
        }

        /*
            withdraw shares from vault, returning the amount of
            assets to be transferred to the receiver
        */
        (uint256 assets, uint256 protocolFee) = _redeem(id, msg.sender, shares);

        // transfer eth to receiver factoring in fees/shares
        (bool success,) = payable(receiver).call{value: assets}("");
        if (!success) {
            revert Errors.MultiVault_TransferFailed();
        }

        _transferFeesToProtocolVault(protocolFee);

        return assets;
    }
function redeemTriple(uint256 shares, address receiver, uint256 id) external nonReentrant returns (uint256) {
        if (!isTripleId(id)) {
            revert Errors.MultiVault_VaultNotTriple();
        }

        /*
            withdraw shares from vault, returning the amount of
            assets to be transferred to the receiver
        */
        (uint256 assets, uint256 protocolFee) = _redeem(id, msg.sender, shares);

        // transfer eth to receiver factoring in fees/shares
        (bool success,) = payable(receiver).call{value: assets}("");
        if (!success) {
            revert Errors.MultiVault_TransferFailed();
        }

        _transferFeesToProtocolVault(protocolFee);

        return assets;
    }

Recommendation

Check for the account's existence prior to transferring.

mihailo-maksa commented 4 months ago

The reported issue concerning the risk of losing funds due to non-existent receiver addresses has been reviewed. Here is our detailed response:

EVM Address Handling: In the Ethereum Virtual Machine (EVM), there is no distinction between existent and non-existent addresses as long as the address format is valid. Ether transfers to any valid address format, regardless of its prior usage, will succeed if the address is correct.

User Responsibility: It is the responsibility of the user to ensure that the receiver address provided is correct and intended. The protocol assumes that users verify their input before initiating transactions, as it cannot differentiate between intended and unintended addresses.

No Practical Risk: The described issue does not pose a practical risk since any valid Ethereum address, whether previously used or not, can receive Ether. Therefore, the transfer will always succeed if the address format is valid, mitigating the concern of losing funds due to a non-existent address.

Conclusion: The current implementation aligns with the standard behavior of the EVM regarding address handling. The risk of losing funds due to non-existent receiver addresses is not a concern as long as the address format is valid. Therefore, we consider this issue to be invalid.

Status: This issue is invalid.