`PALMERA_TX_TYPEHASH` incorrectly calculated

[hats-bug-reporter[bot]]

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

Description:

Description

The constant PALMERA_TX_TYPEHASH in the contract Constants is incorrectly calculated

Attack Scenario

This breaks EIP-721

Attachments

PoC

https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/dfd821e2fd7825c66c079c19be9460238f6e045a/src/libraries/Constants.sol#L16-L20

    /// @dev keccak256(
    ///     "PalmeraTx(address org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)"
    /// );
    bytes32 internal constant PALMERA_TX_TYPEHASH =
        0x5576bff5f05f6e5452f02e4fe418b1519cb08f54fae3564c3a4d2a4706584d4e;

Should be:

    /// @dev keccak256(
    ///     "PalmeraTx(address org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)"
    /// );
    bytes32 internal constant PALMERA_TX_TYPEHASH =
        0x33d86b91ace2c23c833e6a968f94ce2cdabd89ed7375f3d2135aa0f5a9c131b5;

[rotcivegaf]

Fix, it should be:

    /// @dev keccak256(
    ///     "PalmeraTx(bytes32 org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)"
    /// );
    bytes32 internal constant PALMERA_TX_TYPEHASH =
        0xf101cc05719d99662b60575177bd09d54327c4615e26e40a225e9c1365e3fdc9;

[0xRizwan]

This is valid issue. Due to incorrect PALMERA_TX_TYPEHASH, the encoded transaction data for Palmera transactions would be incorrectly produced. Therefore, the transaction hashes requested for Palmera transaction would be incorrectly returned. On side note, this issue does not have description and it does not describe the impact of issue on contracts functionalities. Therefore, this should be considered as low severity.

@0xmahdirostami Let me know your thoughts on severity of this issue.

[alfredolopez80]

i confirm this is a valid issue

Should Be:

/// @dev keccak256(
///     "PalmeraTx(address org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)"
/// );
bytes32 internal constant PALMERA_TX_TYPEHASH =
    0x33d86b91ace2c23c833e6a968f94ce2cdabd89ed7375f3d2135aa0f5a9c131b5;

Additional i agree with @0xRizwan this is a low severity because use that same constant in the encodeTransactionData Function and getTransactionHash.

[rotcivegaf]

Sorry for the basic report but the important thing is speed in these competitions

@alfredolopez80 https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/issues/1#issuecomment-2186816069 this is the correct value

The org is a bytes32 not an address: https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/1ac35880b5d45154267788e2db548eaaae0beaa0/src/Helpers.sol#L72

So, from: PalmeraTx(address org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce) To PalmeraTx(bytes32 org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)

[alfredolopez80]

Sorry for the basic report but the important thing is speed in these competitions

@alfredolopez80 #1 (comment) this is the correct value

The org is a bytes32 not an address:

https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/1ac35880b5d45154267788e2db548eaaae0beaa0/src/Helpers.sol#L72

So, from: PalmeraTx(address org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce) To PalmeraTx(bytes32 org,address superSafe,address targetSafe,address to,uint256 value,bytes data,uint8 operation,uint256 _nonce)

@rotcivegaf yeah you right, and thanks for clarify!!

[rotcivegaf]

I think it's a valid medium

From 2023-07-lens-findings/issues/141:

Due to the use of incorrect typehashes, the signature verification in the functions listed above is not EIP-712 compliant. Contracts or dapps/backends that use "correct" typehashes that match the parameters of these functions will end up generating different signatures, causing them to revert when called.

Basically, as I said in the report, it breaks the EIP-712. Any contract, backend or website that wants to implement this signature must apply a "hack"(manually make the signature) to create the signature otherwise the transactions will revert On the other hand, wallets like Metamask and others will not be able to recognize this signature and will not be able to display the parameters in a user friendly way when signing the message