hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

`registerOrg` Function Vulnerable to DoS and Gas Griefing Attacks #3

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

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

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x3a209da07e47ea408d83f5ada006360c3e100671c7003cd5990f5fd8d705a2bf Severity: medium

Description: Description: The registerOrg function in the contract can be exploited by an attacker to perform denial-of-service (DoS) attacks or gas griefing against other users. The function allows for the creation of organizations, but it doesn't adequately handle the scenario where an organization name is already registered. An attacker can front-run a legitimate user's transaction to create an organization with a desired name, causing the legitimate user's transaction to fail. This forces the user to attempt the process again.

Impact: Denial of Service (DoS) and Gas Griefing

Scenario:

  1. A user wants to create an organization with the name "organizationA".
  2. A malicious user front-runs the transaction and creates an organization with the name "organizationA".
  3. The user's transaction fails due to the name conflict.
  4. The user has to go through the transaction process again, choosing a new name.
  5. This process can be repeated by the attacker, continuously causing the user's transactions to fail.

Proof of Concept (PoC): The registerOrg function calls _createOrgOrRoot, which includes the following logic:

function _createOrgOrRoot(
    string memory name,
    address caller,
    address newRootSafe
) private returns (uint256 safeId) {
    if (bytes(name).length == 0) {
        revert Errors.EmptyName();
    }
    bytes32 org = caller == newRootSafe
        ? bytes32(keccak256(abi.encodePacked(name)))
        : getOrgHashBySafe(caller);
    if (isOrgRegistered(org) && caller == newRootSafe) {
        revert Errors.OrgAlreadyRegistered(org);
    }

Due to the isOrgRegistered check, the function reverts if an organization with the same name is already registered.

Mitigation: To prevent this attack, include the caller's address in the hash when creating an organization. This ensures that organization names are unique to each user, preventing front-running attacks.

Updated _createOrgOrRoot function:

function _createOrgOrRoot(
    string memory name,
    address caller,
    address newRootSafe
) private returns (uint256 safeId) {
    if (bytes(name).length == 0) {
        revert Errors.EmptyName();
    }
    bytes32 org = caller == newRootSafe
        ? bytes32(keccak256(abi.encodePacked(name, caller)))
        : getOrgHashBySafe(caller);
    if (isOrgRegistered(org) && caller == newRootSafe) {
        revert Errors.OrgAlreadyRegistered(org);
    }
    // Additional logic for organization creation
}

By including the caller address in the hash, organization names become unique per user, mitigating the risk of front-running and gas griefing attacks.

0xRizwan commented 1 week ago

Attack scenario described seems to be correct, i think this should be Medium severity.

0xmahdirostami commented 1 week ago

@alfredolopez80 @0xRizwan , thanks. i think the issue is in medium severity due to a doc in competition scope.

0xRizwan commented 5 days ago

Agreed, Historically such issues are considered as Medium severity.