091.md

Tangy Juniper Donkey

Medium

Hash Collisions in getServiceAndAccountHash() Prevent Creation of Legitimate Attestations

Summary:

The use of abi.encodePacked() with multiple dynamic string inputs will cause denial of core functionality for legitimate users as malicious actors can craft colliding service and account combinations that prevent new attestation creation.

Vulnerability Detail

The EthosAttestation contract manages attestations linking user profiles to external web2 services. The contract's core functionality revolves around creating and managing these attestations through the createAttestation() function.

The issue exists in the getServiceAndAccountHash() function:

function getServiceAndAccountHash(
    string calldata service,
    string calldata account
) public pure returns (bytes32) {
    if (bytes(service).length == 0 || bytes(account).length == 0) {
        revert AttestationInvalid(service, account);
    }
    return keccak256(abi.encodePacked(service, account));
}

This function uses abi.encodePacked() to concatenate two dynamic string inputs before hashing. According to Solidity documentation:

"If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c")."

The issue manifests in the following flow:

1. User A creates an attestation with:

service = "serviceA"
account = "accountBC"

2. User B attempts to create a different attestation with:

service = "serviceAB"
account = "accountC"

3. Both combinations produce identical hashes because:

abi.encodePacked("serviceA", "accountBC") == abi.encodePacked("serviceAB", "accountC")
// Both result in "serviceAaccountBC"

4. In createAttestation(), the following checks occur:

bytes32 hashStr = getServiceAndAccountHash(
    attestationDetails.service,
    attestationDetails.account
);

bool isClaimed = _claimAttestation(profileId, hashStr, evidence);
if (isClaimed) {
    return;
}

bool isRestore = restoreIfArchived(hashStr);
if (isRestore) {
    return;
}

_attestationShouldNotExist(hashStr);  // Reverts here due to hash collision

The function reverts at _attestationShouldNotExist() because the hash already exists, preventing User B from creating their legitimate attestation.

Impact

This vulnerability breaks core contract functionality by preventing legitimate users from creating unique attestations when their service/account combination produces a hash that collides with an existing attestation. According to Sherlock's validation criteria, this qualifies as a Medium severity issue as it Breaks core contract functionality, rendering the contract useless or leading to loss of funds for affected users.

Proof of Concept

// Test Case
function testHashCollision() public {
    // User A's attestation
    string memory serviceA = "serviceA";
    string memory accountA = "accountBC";
    
    // User B's attestation
    string memory serviceB = "serviceAB";
    string memory accountB = "accountC";
    
    // Create first attestation (succeeds)
    createAttestation(1, 123, AttestationDetails(serviceA, accountA), "evidence", signature);
    
    // Try to create second attestation (reverts)
    vm.expectRevert();
    createAttestation(2, 456, AttestationDetails(serviceB, accountB), "evidence", signature);
}

Tool used

Manual Review

Recommendation

Replace abi.encodePacked() with abi.encode() to prevent hash collisions:

function getServiceAndAccountHash(
    string calldata service,
    string calldata account
) public pure returns (bytes32) {
    if (bytes(service).length == 0 || bytes(account).length == 0) {
        revert AttestationInvalid(service, account);
    }
-   return keccak256(abi.encodePacked(service, account));
+   return keccak256(abi.encode(service, account));
}