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

0 stars 0 forks source link

Wrong implementation of constructor. #53

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

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

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

Description: Description\ In EIP712:constructor before checking for zero address check they are assigning it's value to external_signer

Attack Scenario\ just a bad practice. consider first checking and then assigning.

Attachments

  1. Proof of Concept (PoC) File

constructor(string memory domainName, string memory version, address signer) EIP712(domainName, version) {
        external_signer = signer;
        require(signer != address(0), "ZERO_SIGNER");
    }
  1. Revised Code File (Optional)

constructor(string memory domainName, string memory version, address signer) EIP712(domainName, version) {
-      external_signer = signer;
-       require(signer != address(0), "ZERO_SIGNER");
+       require(signer != address(0), "ZERO_SIGNER");
+      external_signer = signer;
    }
alex-sumner commented 7 months ago

Whilst stylistically a reasonable point, this is not a bug. Reordering the two lines will make no difference to the outcome, which in both cases is that the transaction reverts.

bahurum commented 7 months ago

Since there is no impact at all, I would say this is informational and not low severity