hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

Storage slots derived from hashes are prone to pre-image attacks #5

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

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

Description:

Description

Storage slots manually constructed using keccak hash of a string are prone to storage slot collision as the pre-images of these hashes are known. Attackers may find a potential path to those storage slots using the keccak hash function in the codebase and some crafted payload

Attachments

PoC

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

    /// @dev keccak256("guard_manager.guard.address")
    bytes32 internal constant GUARD_STORAGE_SLOT =
        0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;

Should be:


    /// @dev keccak256("guard_manager.guard.address") - 1
    bytes32 internal constant GUARD_STORAGE_SLOT =
        0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c7;
0xRizwan commented 1 week ago

Need more context, How would this affect the inscope contracts?

alfredolopez80 commented 1 week ago

Taking into account that SAFE uses the same SLOT and at the moment in both versions, and they have never had an exploit related to this, I consider it invalid

1.3.0: https://github.com/safe-global/safe-smart-account/blob/186a21a74b327f17fc41217a927dea7064f74604/contracts/base/GuardManager.sol#L30

1.4.1: https://github.com/safe-global/safe-smart-account/blob/bf943f80fec5ac647159d26161446ac5d716a294/contracts/base/GuardManager.sol#L42

if you have PoC for this explot pls give us more context!!!