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

Long range attack vulnerability #543

Open
tomaka opened this issue Nov 1, 2021 · 3 comments
Open

Long range attack vulnerability #543

tomaka opened this issue Nov 1, 2021 · 3 comments
Labels
enhancement New feature or request security

Comments

@tomaka
Copy link
Contributor

tomaka commented Nov 1, 2021

The attack goes as follow:

  • A colluding group of validators unbond their tokens.
  • After the end of the unbonding period, this group of validators forks the chain before the moment when they unstaked. They are now on a different chain than the real one.
  • Substrate-connect, at initialization, performs a Grandpa warp sync against a node on this different chain. Substrate-connect is now using on the wrong chain.

In principle, smoldot could detect that there are two chains and send an equivocation report. But because the validators are unbonded on the official chain, they can't actually be punished.

In order to prevent this attack, the checkpoint that smoldot starts syncing from must be more recent than the unbonding period. In other words, we should always sync Kusama from a checkpoint that is less than 7 days old, and we should always sync Polkadot from a checkpoint that is less than 28 days old.
And in addition to this, smoldot should report for equivocations (which it currently doesn't do). cc paritytech/smoldot#698

@tomaka
Copy link
Contributor Author

tomaka commented Nov 1, 2021

In order to prevent this attack, the checkpoint that smoldot starts syncing from must be more recent than the unbonding period. In other words, we should always sync Kusama from a checkpoint that is less than 7 days old, and we should always sync Polkadot from a checkpoint that is less than 28 days old.

This can be done by having the apps self-update periodically to ship a recent version of the chain spec, and having the substrate-connection extension self-update periodically to ship a recent version of the chain spec.

Generating a chain spec requires a fully synced node, and because the chain spec is blindly trusted you shouldn't just use a chain spec generated by a third party unless that third party is fully trusted.
(note that full nodes are also vulnerable to this attack if they have been offline for more than 7 days/28 days, so this node should stay online)

In other words, each app developer (and us, extension developers) should ideally maintain a fully synced node and periodically generate a chain spec and update the app/extension.
This is a really painful thing to do, but it is the only "correct" solution.


One mitigation that we could do is storing the latest finalized block in some sort of local storage (paritytech/smoldot#1565).
At initialization, smoldot would load the finalized block of the last time it was running.

This means that as long as smoldot isn't offline for more than 7 days (Kusama)/28 days (Polkadot), it isn't vulnerable (provided paritytech/smoldot#698 is implemented).

It is likely that the users of the extension will open their browser at least once every 7 days/28 days, so this mitigation would be very efficient for the extension. It is probably less efficient for the SmoldotProvider (the in-page node), though.

This mitigation could also be a double-edged sword, because if the attack is successful, then the next time you load smoldot it will remain on the wrong chain.


Overall I think that:

  • The extension should save the finalized block in some local storage in order to reload it the next time (in other words, the mitigation of the previous section).
  • Parity, as the entity that publishes the browser extension, should update the chain specs and ship updates as often as possible, for example every 3 days. This should of course be done automatically and in a secure way.
  • We should encourage app developers to occasionally ship updates with new chain specs, but without caring too much if they don't.
  • Because this attack is extremely unlikely to happen and its success random, I think it's fine to not deal with it more than that.

@wirednkod wirednkod assigned wirednkod and unassigned wirednkod Nov 1, 2021
@tomaka
Copy link
Contributor Author

tomaka commented Nov 1, 2021

Another possible mitigation could be to do a simple HTTP request to one ore more semi-trusted JSON-RPC nodes at the end of the warp syncing, in order to check whether we're on the same chain as they are.

If there is a mismatch, let the user continue using the app (after all, the entire point of substrate-connect is that it should always work even when JSON-RPC nodes are down/corrupted) but print a very visible warning asking the user to double check what's happening.

I don't think that this is a good solution, though, because we would need to curate a list of JSON-RPC nodes.

@wirednkod wirednkod added enhancement New feature or request security labels Nov 1, 2021
@tomaka
Copy link
Contributor Author

tomaka commented Dec 14, 2021

The extension should save the finalized block in some local storage in order to reload it the next time (in other words, the mitigation of the previous section).

Covered by #568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security
Projects
None yet
Development

No branches or pull requests

2 participants