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

Malicious shard might create hash conflicts in receipts #1838

Closed
evgenykuzyakov opened this issue Dec 9, 2019 · 12 comments
Closed

Malicious shard might create hash conflicts in receipts #1838

evgenykuzyakov opened this issue Dec 9, 2019 · 12 comments
Assignees
Labels
A-chain Area: Chain, client & related A-RPC Area: rpc C-bug Category: This is a bug
Milestone

Comments

@evgenykuzyakov
Copy link
Collaborator

if a shard is completely compromised it can produce a receipt with a conflicting hash. E.g. the hash might conflict either with some other receipt or with a transaction. For a transaction it would trigger failed to decode during borsh. For a receipt we might return a wrong receipt or have receipt conflicts.
Eventually it will be challenged and reverted, but we need to make sure it doesn’t crash the node and we handle it correctly.

@evgenykuzyakov evgenykuzyakov added A-security Area: Security issues A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) labels Dec 9, 2019
@MaksymZavershynskyi MaksymZavershynskyi removed the A-security Area: Security issues label Jan 20, 2020
@MaksymZavershynskyi MaksymZavershynskyi added the P-low Priority: low label Jan 20, 2020
@MaksymZavershynskyi MaksymZavershynskyi added this to the MainNet milestone Jan 20, 2020
@evgenykuzyakov evgenykuzyakov added the A-chain Area: Chain, client & related label Feb 19, 2020
@evgenykuzyakov
Copy link
Collaborator Author

Here is the scenario:

  1. A malicious chunk producer creates an invalid chunk with invalid transition.
  2. This chunk is included in the block and no one challenged this chunk.
  3. A new malicious chunk is produced on top the previous malicious chunk.
  4. A malicious challenge is created for the 2nd malicious chunk. The challenge is legit by itself, but it's built on top of 1st malicious chunk. The 1st chunk may contain any state and any state root, because it was invalid. So the state proof in the challenge is valid.
  5. When every regular node tries to validate a malicious challenge they might crash on any unexpected storage operation.

The 2nd chunk can also contain any invalid transactions or receipts. So the input to the Runtime can be almost arbitrary picked by the malicious nodes.

@MaksymZavershynskyi MaksymZavershynskyi removed the A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) label Feb 26, 2020
@MaksymZavershynskyi
Copy link
Contributor

Assigning to @SkidanovAlex since it might be triggering failure on chain side.

@bowenwang1996
Copy link
Collaborator

Is this fixed in #2155?

@MaksymZavershynskyi
Copy link
Contributor

Is this fixed in #2155?

Assigning to @evgenykuzyakov to provide reply.

@evgenykuzyakov
Copy link
Collaborator Author

It's addressed on Runtime side. I think Runtime is pretty safe, since it doesn't have unwraps or expects and doesn't trust the state at all.
It's likely still an issue on RPC side and maybe on chain side

@bowenwang1996
Copy link
Collaborator

bowenwang1996 commented Feb 27, 2020

I don't think it's an issue on the rpc side given that we prohibit querying receipts. On the chain side I don't see any issue since the content of receipts is not important from chain's perspective.

@evgenykuzyakov
Copy link
Collaborator Author

So if a shard produces an invalid ExecutionOutcome with the result SuccessReceiptId pointing at the transaction itself. It might create the infinite recursion during fetching of get_recursive_transaction_results

@bowenwang1996
Copy link
Collaborator

bowenwang1996 commented Feb 28, 2020

@evgenykuzyakov that's fine. The query will just time out. If you are talking about the node answering the query, since the shard is corrupted I don't think we need to worry about the safety of a malicious node.

@evgenykuzyakov evgenykuzyakov added the A-RPC Area: rpc label Mar 5, 2020
@frol frol removed the P-low Priority: low label Mar 14, 2020
@MaksymZavershynskyi
Copy link
Contributor

@bowenwang1996 AFAIU this attacks honest validator.

@MaksymZavershynskyi MaksymZavershynskyi added the C-bug Category: This is a bug label Mar 16, 2020
@bowenwang1996
Copy link
Collaborator

This can only happen if the shard is controlled by a malicious majority, which means that honest validators, if any, will submit a challenge, and I think it is fine if they are temporarily subject to this attack before the chain confirms the challenge and slash malicious actors. Also moving this to post mainnet milestone as we will not have multiple shards first.

@bowenwang1996 bowenwang1996 modified the milestones: MainNet, Post MainNet Mar 16, 2020
@stale
Copy link

stale bot commented Jul 1, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 1, 2021
@bowenwang1996
Copy link
Collaborator

looks like no action items here. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chain Area: Chain, client & related A-RPC Area: rpc C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

5 participants