Lack of zero check

[hats-bug-reporter[bot]]

Github username: @https://github.com/sekkiat Twitter username: -- Submission hash (on-chain): 0xd3064ee5c12251e0d59ddb10b495019e1a48387128e208c62db9c67fec83856f Severity: low

Description: Description\ Zero checking not implemented in the constructor in Bfx.sol and PoolDeposit.sol

Attack Scenario\ The fund will be locked if set the wrong address.

Attachments

1. Proof of Concept (PoC) File\ The constructor in Bfx.sol does not implement expiration protection for the signed transaction, allowing the attacker to execute the transaction after a long time.

    constructor(address _owner, address _signer, address _paymentToken
        ) EIP712Verifier("BfxWithdrawal", "1", _signer) {
        //@audit Lack of Zero Check
        owner = _owner;
        paymentToken = IERC20(_paymentToken);
}

The constructor in PoolDeposit.sol does not implement expiration protection for the signed transaction, allowing the attacker to execute the transaction after a long time.

  constructor(address _owner, address _rabbit, address _paymentToken) {
        //@audit-issue [L-1] Lack of Zero Check
        owner = _owner;
        rabbit = _rabbit;
        paymentToken = IERC20(_paymentToken);
  }

Remediation

• Implement a check on the owner address
• Implement a minimum check on the deposit value.
• Limit the number of deposit request per address.

[alex-sumner]

This is not a bug. The contract will only function correctly if deployed with the correct addresses. Deploying with any other address will not work, and the zero address is just one of the very large number of incorrect addresses that could be supplied. Checking for it is a practice that started with older Solidity versions where missing parameters might plausibly lead to a zero value being inferred. This is no longer the case. See, for example, https://forum.openzeppelin.com/t/removing-address-0x0-checks-from-openzeppelin-contracts/2222