search

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

0 stars 0 forks source link

Missing approved to zero first. #41

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

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

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

Description: Description\ BfxVault._doDeposit function do make the first approve to first that prevents accidental or unauthorized transfers if the token owner forgot to revoke the previous allowance. for example some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value.They must first be approved by zero and then the actual allowance must be approved

Proof of Concept (PoC) File https://github.com/BlastFutures/Blast-Futures-Exchange/blob/e05c71bff53dcd9172bc714d0f9ddb2f403e23e1/foundry/src/BfxVault.sol#L230

function _doDeposit(uint256 amount) internal {
        paymentToken.approve(address(bfx), amount);
        bfx.deposit(amount);
    }

Recommendation First approve bfx address to amount zero.

function _doDeposit(uint256 amount) internal {
  +    paymentToken.approve(address(bfx), 0);
        paymentToken.approve(address(bfx), amount);
        bfx.deposit(amount);
    }
alex-sumner commented 9 months ago

The contract does not provide workarounds for tokens with non-standard approval implementation, such as USDT (which is not currently deployed on Blast). This is by design because there is no intention to work with such tokens.