Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The timecache should be checked/updated after validation #155

Closed
vyzo opened this issue Jan 21, 2019 · 6 comments
Closed

The timecache should be checked/updated after validation #155

vyzo opened this issue Jan 21, 2019 · 6 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@vyzo
Copy link
Collaborator

vyzo commented Jan 21, 2019

Currently, the timecache is checked/updated for seen messages prior to message validation.
This was done with the intention of avoiding duplicate message validation.
Unfortunately it introduces a censorship attack vector:
If an adversarial actor can predict a peer's sequence number (by observing a prior message), then it can attempt to send an invalid message with the predicted id.
The message will fail validation, due to signature firstly; but it will also poison the timecache, resulting in dropping a sebsquent message from the peer with the predicted id.

cc @whyrusleeping @Stebalien

@vyzo vyzo added the kind/bug A bug in existing code (including security flaws) label Jan 21, 2019
@vyzo
Copy link
Collaborator Author

vyzo commented Jan 21, 2019

Proposed action: move the timecache check/update after validation.
It will result in duplicate validation of seen messages, but it will nullify the attack vector.

@vyzo
Copy link
Collaborator Author

vyzo commented Jan 21, 2019

In addition to the censorship attack vector, an adversarial actor can cause peers to consume memory in the timecache by sending a torrent of invalid messages.
The fix in #155 also closes this vector.

@Stebalien
Copy link
Member

SGTM. We could also check in both places to avoid re-validating duplicate valid messages.

@Stebalien
Copy link
Member

We could also have a separate (shorter) validation cache where we cache a hash of the message/author.

@vyzo
Copy link
Collaborator Author

vyzo commented Jan 21, 2019

We can do with a check of the valid message cache, but without update.

@vyzo
Copy link
Collaborator Author

vyzo commented Jan 21, 2019

Note that the hash cache won't work any better, as the author cannot be proven to be authentic prior to validation.

@vyzo vyzo closed this as completed in #156 Jan 21, 2019
@ghost ghost removed the in progress label Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

2 participants