code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MetaTxLib.sol#L450-L453

Vulnerability details

Bug Description

In MetaTxLib.sol, the domain seperator for the LensHub contract on Polygon is precomputed and stored as a constant:

    /**
     * @dev We store the domain separator and LensHub Proxy address as constants to save gas.
     *
     * keccak256(
     *     abi.encode(
     *         keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'),
     *         keccak256('Lens Protocol Profiles'), // Contract Name
     *         keccak256('2'), // Version Hash
     *         137, // Polygon Chain ID
     *         address(0xDb46d1Dc155634FbC732f92E853b10B288AD5a1d) // Verifying Contract Address - LensHub Address
     *     )
     * );
     */
    bytes32 constant LENS_HUB_CACHED_POLYGON_DOMAIN_SEPARATOR =
        0xbf9544cf7d7a0338fc4f071be35409a61e51e9caef559305410ad74e16a05f2d;

It is then used in calculateDomainSeparator() as the EIP-712 domain separator when verifying signatures for the LensHub contract:

MetaTxLib.sol#L450-L453

    function calculateDomainSeparator() internal view returns (bytes32) {
        if (address(this) == LENS_HUB_ADDRESS) {
            return LENS_HUB_CACHED_POLYGON_DOMAIN_SEPARATOR;
        }

However, if Polygon ever hard forks, signature replay becomes possible across the original and forked chain. This is because LENS_HUB_CACHED_POLYGON_DOMAIN_SEPARATOR (which contains block.chainid) is pre-computed and will remain the same on the forked chain, even though its chain ID is different.

Impact

If a chain hard forks, signature replay becomes possible across the original and forked chain. As MetaTxLib is mainly used to verify signatures for meta-transactions, such as postWithSig(), signatures that are used to execute such functions on one chain will become replayable on the other chain.

Recommended Mitigation

Consider checking if block.chainid matches Polygon's chain ID before using LENS_HUB_CACHED_POLYGON_DOMAIN_SEPARATOR as the domain seperator:

uint256 constant POLYGON_CHAIN_ID = 137;

MetaTxLib.sol#L450-L453

    function calculateDomainSeparator() internal view returns (bytes32) {
-       if (address(this) == LENS_HUB_ADDRESS) {
+       if (address(this) == LENS_HUB_ADDRESS && block.chainid == POLYGON_CHAIN_ID) {
            return LENS_HUB_CACHED_POLYGON_DOMAIN_SEPARATOR;
        }

Assessed type

Context

donosonaumczuk commented 1 year ago

We accept it but we are not sure about the severity here, Medium might be too much.

c4-sponsor commented 1 year ago

donosonaumczuk marked the issue as disagree with severity

Picodes commented 1 year ago

Interesting finding. I think it is of QA severity as the protocol is not multichain, so replay attacks on the "forked copy" of Lens aren't really an issue

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

code-423n4 / 2023-07-lens-findings