Open sherlock-admin3 opened 2 weeks ago
0xSolus
Medium
According to the team, archive profiles should only allow to restore their profile. But, this invariant is broken in multiple scenarios.
No response
The following functions are not restricted to archive profiles:
EthosProfile::uninvateUser()-> allows to uninvitate a user.
EthosProfile::uninvateUser()
EthosReview::archiveReview() -> allows to archives a review.
EthosReview::archiveReview()
// test/EthosProfile.test.ts describe('archive profiles should only be able to restore their profile', () => { it('uninviteUser', async () => { const { ethosProfile, PROFILE_CREATOR_0, OWNER } = await loadFixture(deployFixture); await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address); await ethosProfile.connect(OWNER).archiveProfile(); await expect(ethosProfile.connect(OWNER).uninviteUser(PROFILE_CREATOR_0.address)).to.be.reverted; }); it('archiveReview', async () => { const { ethosProfile, ethosReview, PROFILE_CREATOR_0, OWNER } = await loadFixture(deployFixture); await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address); await ethosReview.connect(OWNER).addReview( 0, '0xD6d547791DF4c5f319F498dc2d706630aBE3e36f', '0x0000000000000000000000000000000000000000', 'this is a comment', 'this is metadata', { account: '', service: '' }, ); await ethosProfile.connect(OWNER).archiveProfile(); await expect(ethosReview.connect(OWNER).archiveReview(0)).to.be.reverted; }); });
EthosAttestation::claimAttestation() -> allows only to claim previously created Attestation.
EthosAttestation::claimAttestation()
EthosAttestation::archiveAttestation() -> allows to archive an attestation.
EthosAttestation::archiveAttestation()
// test/EthosAttestations.test.ts describe('archive profiles should only be able to restore their profile', () => { it('previously created claimAttestation', async () => { const { OWNER, PROFILE_CREATOR_0, ethosProfile, ethosAttestation, SERVICE_X, ACCOUNT_NAME_BEN, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, } = await loadFixture(deployFixture); await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address); await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1); await ethosProfile.connect(OWNER).archiveProfile(); const creator0profileId = String( await ethosProfile.profileIdByAddress(PROFILE_CREATOR_0.address), ); const ownerprofileId = String( await ethosProfile.profileIdByAddress(OWNER.address), ); const randValue = '123'; const signature = await common.signatureForCreateAttestation( creator0profileId, randValue, ACCOUNT_NAME_BEN, SERVICE_X, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, ); const signatureOwner = await common.signatureForCreateAttestation( ownerprofileId, randValue, ACCOUNT_NAME_BEN, SERVICE_X, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, ); const attestationHash = await ethosAttestation.getServiceAndAccountHash( SERVICE_X, ACCOUNT_NAME_BEN, ); expect( await ethosAttestation.getAttestationHashesByProfileId(creator0profileId), ).to.deep.equal([], 'Wrong attestationHashesByProfileId before'); await ethosAttestation .connect(PROFILE_CREATOR_0) .createAttestation( creator0profileId, randValue, { account: ACCOUNT_NAME_BEN, service: SERVICE_X }, ATTESTATION_EVIDENCE_0, signature, ); expect( await ethosAttestation.getAttestationHashesByProfileId(creator0profileId), ).to.deep.equal([attestationHash], 'Wrong attestationHashesByProfileId after'); await ethosAttestation .connect(OWNER) .createAttestation( ownerprofileId, randValue, { account: ACCOUNT_NAME_BEN, service: SERVICE_X }, ATTESTATION_EVIDENCE_0, signatureOwner, ); expect( await ethosAttestation.getAttestationHashesByProfileId(ownerprofileId), ).to.deep.equal([attestationHash], 'Wrong attestationHashesByProfileId after'); }); it('allows to archive an attestation', async () => { const { OWNER, PROFILE_CREATOR_0, ethosProfile, ethosAttestation, SERVICE_X, ACCOUNT_NAME_BEN, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, } = await loadFixture(deployFixture); await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address); const ownerprofileId = String( await ethosProfile.profileIdByAddress(OWNER.address), ); const randValue = '123'; const signatureOwner = await common.signatureForCreateAttestation( ownerprofileId, randValue, ACCOUNT_NAME_BEN, SERVICE_X, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, ); const attestationHash = await ethosAttestation.getServiceAndAccountHash( SERVICE_X, ACCOUNT_NAME_BEN, ); await ethosAttestation .connect(OWNER) .createAttestation( ownerprofileId, randValue, { account: ACCOUNT_NAME_BEN, service: SERVICE_X }, ATTESTATION_EVIDENCE_0, signatureOwner, ); expect( await ethosAttestation.getAttestationHashesByProfileId(ownerprofileId), ).to.deep.equal([attestationHash], 'Wrong attestationHashesByProfileId after'); await ethosProfile.connect(OWNER).archiveProfile(); await ethosAttestation.connect(OWNER).archiveAttestation(attestationHash); const attestation = await ethosAttestation.connect(OWNER).getAttestationByHash(attestationHash); expect(attestation.archived).to.be.eq(true); }); });
Consider restricting those functions to archive profiles. Using verifiedProfileIdForAddress(addr) will revert in the case of an archive profile.
verifiedProfileIdForAddress(addr)
0xSolus
Medium
Archives profile should only be allowed to restore their profile.
Summary
According to the team, archive profiles should only allow to restore their profile. But, this invariant is broken in multiple scenarios.
Root Cause
No response
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
The following functions are not restricted to archive profiles:
PoC
EthosProfile::uninvateUser()
-> allows to uninvitate a user.EthosReview::archiveReview()
-> allows to archives a review.EthosAttestation::claimAttestation()
-> allows only to claim previously created Attestation.EthosAttestation::archiveAttestation()
-> allows to archive an attestation.Mitigation
Consider restricting those functions to archive profiles. Using
verifiedProfileIdForAddress(addr)
will revert in the case of an archive profile.