BfxVault::stake allows burning tokens to zero address if some special tokens are used

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

Description: Description\ BfxVault::stake function doesnt have checks to prevent sending tokens to zero address assuming this check will be performed by paymentToken, however some tokens dont perform this check leading to token burning

Attack Scenario\ If some special token like DAI is used as paymentToken and bfx address is set to zero address then DAI tokens will be burned. In this scenario owner set bfx address to zero address and DAI as paymentToken, then when some user use BfxVault will lead to token burning

Attachments

Proof of Concept (PoC) File In this PoC owner set Bfx address to zero, and changes token to DAI. So, when a user calls BfxVault::stake his tokens are burned. Append this test case in BfxVault.t.sol:


function testBfxTokenLostToZeroAddress() public {
    // amount
    uint amount = 1234567;
    address user = address(0x123abc);

    IERC20 _daitoken; 
    _daitoken = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);

    // change bfxAddressTo Zero  
    vm.prank(_vaultOwner);
    _vault.setBfx(address(0));

    // change vault payment token
    vm.prank(_vaultOwner);
    _vault.setPaymentToken(address(_daitoken));

    // set DAI balance to user
    deal(address(_daitoken), user, amount, true);
    console.log("Balance before staking DAI to zero address");
    uint dai_zero_address_balance_before = _daitoken.balanceOf(address(0));
    console.log("_daitoken.balanceOf(user) ",_daitoken.balanceOf(user));
    console.log("_daitoken.balanceOf(address(0)) ",dai_zero_address_balance_before);

    // stake
    vm.startPrank(user);
    _daitoken.approve(address(_vault), amount);
    vm.expectEmit(true, false, false, true, address(_vault));
    emit Stake(1, user, amount);
    _vault.stake(amount);
    vm.stopPrank();

    // log balances
    console.log("Balance after staking DAI to zero address");
    console.log("_daitoken.balanceOf(user) ",_daitoken.balanceOf(user));
    uint dai_zero_address_balance_after =_daitoken.balanceOf(address(0));
    console.log("_daitoken.balanceOf(address(0)) ",dai_zero_address_balance_after);
    // check balances
    assertEq(_daitoken.balanceOf(user), 0);
    assertEq(_daitoken.balanceOf(address(0)), dai_zero_address_balance_before+amount);
}

Run this test using a mainnet fork:

forge test --fork-url https://rpc.ankr.com/eth  --match-contract BfxVaultV2 -vvv

Revised Code File (Optional)

Files:

BurnToken.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmYWu7fGBacyas5kot1wyxsFA2HSh1tas7GdmEwxz3XcMZ)
BfxVaultV2.t.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmY1ovqKM7gTCWpEvcvZWHCPnxyYP47NsUN2JyLBQ5UpAx)

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

BfxVault::stake allows burning tokens to zero address if some special tokens are used #59