Skip to content

Commit

Permalink
fix: validate that fid on username add message matches fid on ens pro…
Browse files Browse the repository at this point in the history
…of (#2378)

This fixes a bug which allows an users to assume an arbitrary user's ens
name as their own.

## Merge Checklist

_Choose all relevant options below by adding an `x` now or at any time
before submitting for review_

- [x] PR title adheres to the [conventional
commits](https://www.conventionalcommits.org/en/v1.0.0/) standard
- [x] PR has a
[changeset](https://github.com/farcasterxyz/hub-monorepo/blob/main/CONTRIBUTING.md#35-adding-changesets)
- [x] PR has been tagged with a change label(s) (i.e. documentation,
feature, bugfix, or chore)
- [ ] PR includes
[documentation](https://github.com/farcasterxyz/hub-monorepo/blob/main/CONTRIBUTING.md#32-writing-docs)
if necessary.


<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on enhancing the validation of `fid` in user data
messages to ensure that it matches the `fid` associated with the
username proof, specifically for `fname` and `ENS` types.

### Detailed summary
- Updated validation logic in `index.ts` to check if `fid` matches for
`fname` and `ENS` username types.
- Added error handling for invalid username types.
- Introduced a new test case in `index.test.ts` to verify failure when
`fid` on message does not match `fid` on ENS name proof.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your
question}`

<!-- end pr-codex -->
  • Loading branch information
aditiharini authored Oct 24, 2024
1 parent a56ed67 commit c2140e2
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/warm-cheetahs-drive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@farcaster/hubble": patch
---

fix: validate that fid on username add message matches fid on ens proof
37 changes: 37 additions & 0 deletions apps/hubble/src/storage/engine/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,43 @@ describe("mergeMessage", () => {
expect(result._unsafeUnwrapErr().message).toMatch("failed to resolve ens");
});

test("fails when fid on message doesn't match fid on ens name proof", async () => {
const fid2 = Factories.Fid.build();
const signer2 = Factories.Ed25519Signer.build();
const custodySigner2 = Factories.Eip712Signer.build();
const signerKey2 = (await signer2.getSignerKey())._unsafeUnwrap();
const custodySignerKey2 = (await custodySigner2.getSignerKey())._unsafeUnwrap();
const custodyEvent2 = Factories.IdRegistryOnChainEvent.build(
{ fid: fid2 },
{ transient: { to: custodySignerKey2 } },
);
const signerAddEvent2 = Factories.SignerOnChainEvent.build(
{ fid: fid2 },
{ transient: { signer: signerKey2 } },
);
const storageEvent2 = Factories.StorageRentOnChainEvent.build({ fid: fid2 });
await engine.mergeOnChainEvent(custodyEvent2);
await engine.mergeOnChainEvent(signerAddEvent2);
await engine.mergeOnChainEvent(storageEvent2);

const spoofedUserDataAdd = await Factories.UserDataAddMessage.create(
{
data: {
fid: fid2,
userDataBody: {
type: UserDataType.USERNAME,
value: "test123.eth",
},
},
},
{ transient: { signer: signer2 } },
);

const result = await engine.mergeMessage(spoofedUserDataAdd);
expect(result).toMatchObject(err({ errCode: "bad_request.validation_failure" }));
expect(result._unsafeUnwrapErr().message).toMatch(`fid ${fid} does not match message fid ${fid2}`);
});

test("revokes the user data add when the username proof is revoked", async () => {
const result = await engine.mergeMessage(userDataAdd);
expect(result.isOk()).toBeTruthy();
Expand Down
31 changes: 18 additions & 13 deletions apps/hubble/src/storage/engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1279,23 +1279,28 @@ class Engine extends TypedEmitter<EngineEvents> {
return err(nameProof.error);
}

if (nameProof.value.type === UserNameType.USERNAME_TYPE_FNAME) {
// Check that the fid for the fname and message are the same
if (nameProof.value.fid !== message.data.fid) {
return err(
new HubError(
"bad_request.validation_failure",
`fname fid ${nameProof.value.fid} does not match message fid ${message.data.fid}`,
),
);
}
} else if (nameProof.value.type === UserNameType.USERNAME_TYPE_ENS_L1) {
if (
nameProof.value.type !== UserNameType.USERNAME_TYPE_FNAME &&
nameProof.value.type !== UserNameType.USERNAME_TYPE_ENS_L1
) {
return err(new HubError("bad_request.validation_failure", "invalid username type"));
}

// Check that the fid for the fname/ens name and message are the same
if (nameProof.value.fid !== message.data.fid) {
return err(
new HubError(
"bad_request.validation_failure",
`fid ${nameProof.value.fid} does not match message fid ${message.data.fid}`,
),
);
}

if (nameProof.value.type === UserNameType.USERNAME_TYPE_ENS_L1) {
const result = await this.validateEnsUsernameProof(nameProof.value, custodyAddress);
if (result.isErr()) {
return err(result.error);
}
} else {
return err(new HubError("bad_request.validation_failure", "invalid username type"));
}
}
}
Expand Down

0 comments on commit c2140e2

Please sign in to comment.