Skip to content

Check for malformed ctx.from ids in clients#379

Merged
Monkatraz merged 1 commit into
mainfrom
brn-check-for-broken-from-id-from-clients
May 31, 2026
Merged

Check for malformed ctx.from ids in clients#379
Monkatraz merged 1 commit into
mainfrom
brn-check-for-broken-from-id-from-clients

Conversation

@Monkatraz
Copy link
Copy Markdown
Contributor

@Monkatraz Monkatraz commented May 31, 2026

Why

A broken client could send messages as the wrong from and cause things to break.

What changed

  • Made it so that sessions drop clients who send messages with the wrong from.
  • Made it so that during validate, a server can check the transport identity and ensure binding of the ID is valid.

Versioning

  • Breaking protocol change
  • Breaking ts/js API change

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Monkatraz Monkatraz marked this pull request as ready for review May 31, 2026 22:57
@Monkatraz Monkatraz requested a review from a team as a code owner May 31, 2026 22:57
@Monkatraz Monkatraz requested review from darshkpatel and removed request for a team May 31, 2026 22:57
@Monkatraz Monkatraz force-pushed the brn-check-for-broken-from-id-from-clients branch from 36b3b15 to 24ff025 Compare May 31, 2026 23:03
@Monkatraz Monkatraz merged commit df4ce25 into main May 31, 2026
7 checks passed
@Monkatraz Monkatraz deleted the brn-check-for-broken-from-id-from-clients branch May 31, 2026 23:09
Copy link
Copy Markdown
Contributor

@lhchavez lhchavez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but we should try to remove those fields

const parsedMsg = parsedMsgRes.value;

// messages must originate from this session's peer
if (parsedMsg.from !== this.to) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we even allow connections to specify the to/from? i think this is a vestigial feature that we never ended up using.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you're correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants