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

0 stars 0 forks source link

Vulnerability in makeDeposit Function Allows Unauthorized Token Approvals #52

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

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

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

Description: Description\ In the BfxVault:makeDeposit function, there's a vulnerability related to the lack of a check for the actual token balance before approving the transfer of funds to the BFX exchange contract. The function assumes that the caller has the necessary funds available, but it does not verify this condition explicitly.

Attack Scenario\ ->suppose the malacious Admin give access of TREASURER_ROLE to other attacker then they can corrupt the system.

Malicious Approval: An attacker, who may have malicious intent or simply wants to disrupt the system, calls the makeDeposit function with an arbitrary amount that exceeds the actual token balance of the contract.

Unverified Approval: Since the function does not check the token balance before approving the transfer, it proceeds to approve the specified amount without verifying if the funds are actually available.

Unauthorized Transfer: The attacker successfully obtains approval for a transfer that exceeds the contract's token balance. This could lead to a situation where the contract attempts to transfer funds it doesn't possess, resulting in an unauthorized transfer and potential loss of funds.

Exploitation: The attacker may exploit this vulnerability to bypass any intended security measures or to cause financial harm to the system by approving fraudulent transfers or triggering unexpected behavior due to insufficient funds.

Attachments

  1. Proof of Concept (PoC) File
  function makeDeposit(uint256 amount) external {
        require(signers[msg.sender][TREASURER_ROLE], "NOT_A_TREASURER");
        _doDeposit(amount);
    }
//@audit-issue explicitly not checking approved amount is legit or not paymentToken.approve(address(bfx),amount)
    function _doDeposit(uint256 amount) internal {
        paymentToken.approve(address(bfx), amount);
        bfx.deposit(amount);
    }

these are blast(USDB) token approve function even they don;t have check whether contract have that amount of tokens or not.

    function approve(address spender, uint256 amount)
        public
        virtual
        returns (bool)
    {
        address owner = msg.sender;
        _approve(owner, spender, amount);
        return true;
    }

  function _approve(
        address owner,
        address spender,
        uint256 amount
    ) internal override {
        if (owner == address(0)) revert ApproveFromZeroAddress();
        if (spender == address(0)) revert ApproveToZeroAddress();

        _allowances[owner][spender] = amount;
        emit Approval(owner, spender, amount);
    }
  1. Revised Code File (Optional) Includes a makeDeposit function that implements proper checks to ensure that the approved amount does not exceed the actual token balance of the contract.
alex-sumner commented 8 months ago

Transfers can fail and this is correctly handled. No credit will be awarded for a failed transfer and the transaction is reverted. It is not necessary to first check that sufficient tokens are available.

recursiveEth commented 8 months ago

Hey, the above approve function is of Blast contract methods , they don't have any check whether there sufficient amount which they have approved.