hats-finance / Blast-Futures-Exchange-0x97895c329b950755566ddcdad3395caaea395074

0 stars 0 forks source link

Vulnerability in individualDeposit Function Allows Misappropriation of Funds #45

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

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

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

Description: Description\ In PoolDeposit:individualDeposit() 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 individualDeposit Handles individual deposits by transferring the specified amount of tokens from the contributor to the rabbit address.

Parameters:

contributor: Address making the deposit.

amount: Amount of tokens to deposit.

Attack Scenario\ Example: ->Alice and Bob are two user ->Bob calls PoolDeposit:individualDeposit() and in contributor address he passes Alice address. ->msg.sender in above call is Bob and contributor 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 individualDeposit(address contributor, uint256 amount) external {
      require(amount > 0, "WRONG_AMOUNT");
      uint256 depositId = allocateDepositId(); //1e16
      emit Deposit(depositId, contributor, amount, 0);
      bool success = makeTransferFrom(msg.sender, rabbit, amount);
      require(success, "TRANSFER_FAILED");
  }
  1. Revised Code File (Optional)
 function individualDeposit(address contributor, uint256 amount) external {
        require(amount > 0, "WRONG_AMOUNT");
+       // there shd be require statement contributor shd be msg.sender
+       require(contributor == msg.sender,"Invalid contributor")
        uint256 depositId = allocateDepositId(); //1e16
        emit Deposit(depositId, contributor, amount, 0);
        // 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 commented 4 months ago

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.