Skip to content

CommonClient: Prevent disconnect from being sent bad DeathLink data - #6376

Merged
Berserker66 merged 1 commit into
ArchipelagoMW:mainfrom
colin969:cc-deathlink
Aug 7, 2026
Merged

CommonClient: Prevent disconnect from being sent bad DeathLink data#6376
Berserker66 merged 1 commit into
ArchipelagoMW:mainfrom
colin969:cc-deathlink

Conversation

@colin969

@colin969 colin969 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

What is this fixing or adding?

Some clients send incorrectly made DeathLink packets, usually by forgetting to add the timestamp.

This prevents CommonClient from disconnecting when trying to handle missing data or time fields in a DeathLink packet sent by a another client. Bad packets are ignored.

This doesn't do type checking of fields, but I'd argue that shouldn't really happen outside of a malicious client. Feel free to add that as well.

How was this tested?

Connect a deathlink game, open another websocket and deliberately send the packets described in the linked issue.

Closes #6375

@github-actions github-actions Bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Aug 4, 2026
@Berserker66

Copy link
Copy Markdown
Member

So.. this is an interesting problem. By no longer crashing and silently ignoring the issue even more custom clients will construct a wrong deathlink package in the future. On the other hand, leaving it as is does allow a malicous actor to snipe commonclients and force them to disconnect.

@colin969

colin969 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

In fairness, nobody else dying when you do would give that away.

Clients don't get told who sent it either, so there's not much they can do other than ignore it. It'd need to be a server enforced thing and I figure there's reasons not to want to do it there.

Fwiw this covers the common mistakes, malicious actors have better ways of crashing CommonClient already.

@silasary silasary left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm content to say that "Nobody else is dying when I send a DL" is enough of a sign that the client is doing something wrong.

@Berserker66
Berserker66 merged commit 3359cac into ArchipelagoMW:main Aug 7, 2026
3 checks passed
@github-actions github-actions Bot removed the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Aug 7, 2026
@colin969
colin969 deleted the cc-deathlink branch August 8, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects: core Issues/PRs that touch core and may need additional validation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: CommonClient does not validate DeathLink packets before accessing data

3 participants