hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 4 forks source link

use deprecated `this` in calculating `domainSeparator()` #28

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x2a3b4ace63d3b22f2e6028dbbb3c507ca3448da906cc57c4779129556d6f951a Severity: low

Description: Description\

In Helpers.sol, domainSeparator() is calculated as:

    function domainSeparator() public view returns (bytes32) {
        return keccak256(
            abi.encode(Constants.DOMAIN_SEPARATOR_TYPEHASH, getChainId(), this)
        );
    }

The issue is that, this keyword is used for the smart contract when the solidity version is below 0.5.0, therefore use of this is deprecated in v5.0.0 .

The contracts have used solidity 0.8.23 so use of deprecated this should be avoided to prevent unexpected behaviour in contracts. Its always recommended to not use deprecated varibales/functions, etc.

Recommendation to fix\ With solidity 0.8.23, use address(this), instead of this.

    function domainSeparator() public view returns (bytes32) {
        return keccak256(
-            abi.encode(Constants.DOMAIN_SEPARATOR_TYPEHASH, getChainId(), this)
+            abi.encode(Constants.DOMAIN_SEPARATOR_TYPEHASH, getChainId(), address(this))
        );
    }
0xRizwan commented 5 months ago

this is deprecated and replaced with address(this). Openzeppelin also uses address(this) while computing domainSeparator which can be checked here

@alfredolopez80 Can you please check it.

alfredolopez80 commented 4 months ago

is a issue but low