search

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

0 stars 0 forks source link

Missing approved to zero first. #40

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): 0x67128e78b392600f66ea39a98782620bdafcace8dfea3a7d7b0a7aad4eecdf8f 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 4 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.