Vulnerability in pooledDeposit Function Allows Misappropriation of Funds through Impersonation

[hats-bug-reporter[bot]]

Github username: -- Twitter username: @recursiveAudit Submission hash (on-chain): 0xdc4c563b13ebcebd7a558df1c5dd3a4917e84ab4e51d486623be04def381667c Severity: high

Description: Description\ In PoolDeposit:pooledDeposit() that allows a contributor to deposit a certain amount of funds. The function takes two parameters: contributor, which is the address of the contributor, and amount, which is the amount being deposited. The function emits a Deposit event with information about the deposit, including the contributor's address.

However, there's a vulnerability in the code where the makeTransferFrom function is called. This function is supposed to transfer funds from the contributor's address to another address (rabbit). But it uses msg.sender instead of contributor, which means the funds are being transferred from the wrong address. This vulnerability can be exploited by an attacker to manipulate funds.

//document provided by the protocol pooledDeposit Processes pooled deposits from multiple contributors in a single transaction, aggregating their contributions and transferring the total amount to the rabbit address.

Parameters: contributions: An array of Contribution structs, each containing a contributor address and amount.

Attack Scenario\ Example: ->Alice and Bob are two user ->Bob calls PoolDeposit:pooledDeposit() and in contributor address he passes Alice address. ->msg.sender in above call is Bob and contributor.address is Alice address which allow Bob to send misappropriates funds by depositing them from their own address while pretending to be another contributor(Alice). ->This can lead to financial losses and undermine the integrity of the system.

Attachments

1. Proof of Concept (PoC) File

     function pooledDeposit(Contribution[] calldata contributions) external {
        uint256 poolId = allocatePoolId();
        uint256 totalAmount = 0;
        for (uint i = 0; i < contributions.length; i++) {
            Contribution calldata contribution = contributions[i];
            uint256 contribAmount = contribution.amount;
            totalAmount += contribAmount;
            require(contribAmount > 0, "WRONG_AMOUNT");
            require(totalAmount >= contribAmount, "INTEGRITY_OVERFLOW_ERROR");
            uint256 depositId = allocateDepositId();
            emit Deposit(depositId, contribution.contributor, contribAmount, poolId); //
        }
        require(totalAmount > 0, "WRONG_AMOUNT");
        emit PooledDeposit(poolId, totalAmount);
        bool success = makeTransferFrom(msg.sender, rabbit, totalAmount);
        require(success, "TRANSFER_FAILED");
    }

2. Revised Code File (Optional)

      function pooledDeposit(Contribution[] calldata contributions) external {
        uint256 poolId = allocatePoolId();
+       // there shd be require statement contributor shd be msg.sender
+       require(contributions.address == msg.sender,"Invalid contributor")
        uint256 totalAmount = 0;
        for (uint i = 0; i < contributions.length; i++) {
            Contribution calldata contribution = contributions[i];
            uint256 contribAmount = contribution.amount;
            totalAmount += contribAmount;
            require(contribAmount > 0, "WRONG_AMOUNT");
            require(totalAmount >= contribAmount, "INTEGRITY_OVERFLOW_ERROR");
            uint256 depositId = allocateDepositId();
            emit Deposit(depositId, contribution.contributor, contribAmount, poolId); //
        }
        require(totalAmount > 0, "WRONG_AMOUNT");
        emit PooledDeposit(poolId, totalAmount);
+         // or you can change from address in msg.sender to contributor address
+        bool success = makeTransferFrom(msg.sender, rabbit, amount);
-        bool success = makeTransferFrom(msg.sender, rabbit, amount);
        require(success, "TRANSFER_FAILED");
    }

[alex-sumner]

The purpose of the contract is to allow third parties to make deposits on behalf of users. A typical case is that a third party provides a fiat on-ramp - accepting credit or debit card payments from customers and in return depositing funds to the exchange on their behalf. There is no vulnerability here. Bob can indeed transfer funds that are credited to Alice, but that is the intended use case.