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

[Bug] A malicious validator can broadcast certificate with Transaction::Fee and block the network #2990

Closed
feezybabee opened this issue Jan 11, 2024 · 9 comments · Fixed by #3104
Labels
bug Incorrect or unexpected behavior

Comments

@feezybabee
Copy link

https://hackerone.com/reports/2310746

Summary:

Currently, Aleo does not speculate on Transaction::Fee:

                    // There are no finalize operations here.
                    // Note: This will abort the entire atomic batch.
                    Transaction::Fee(..) => Err("Cannot speculate on a fee transaction".to_string()),

However, malicious can propose a BatchPropose with Fee transaction. Other validators won't check if a BatchPropose contains a Fee transaction and will sign the proposal. Once the Certificate with Fee transaction is committed, it will block the execution of the block. All the validators will be blocked.

Steps To Reproduce:

  1. Clone snarkOS
  2. Checkout testnet3: git checkout 052457bddbf736f88227c543646890f8588a0efb
  3. Apply the patch unchecked_fee_transaction.patch in the attachment: git apply unchecked_fee_transaction.patch
  4. Run ./devnet.sh (To make it easy to reproduce, run all 4 validators with the patch)
  5. Wait some round and find the network stuck:
    image

Also see the attached sample log sample_log.zip
sample_log.zip

Proof-of-Concept (PoC)

See above

Fix Suggestions:

Check if a BatchPropose contains Transaction::Fee before signing.

Impact

A malicious validator can broadcast certificate with Transaction::Fee and block the whole network

@feezybabee feezybabee added the bug Incorrect or unexpected behavior label Jan 11, 2024
@randomsleep
Copy link
Contributor

@feezybabee It seems that unchecked_fee_transaction.patch is not attached. Let me put it here:
unchecked_fee_transaction.patch

@vicsn
Copy link
Collaborator

vicsn commented Jan 12, 2024

This looks legit, great work @randomsleep . @ljedrz do you have time coming week to test and propose the fix? See also #2991

@ljedrz
Copy link
Collaborator

ljedrz commented Jan 15, 2024

I can reproduce the issue with the attached patch.

@raychu86
Copy link
Contributor

Please try to reproduce this bug with the changes here - #3083

@ljedrz
Copy link
Collaborator

ljedrz commented Feb 12, 2024

@raychu86 I tried to reproduce it with the mainnet branch, your PR, and the patch attached to this issue, and the network no longer halts; however, I'll retry it with a larger committee now, as a 4-member one had difficulty choosing a leader with the patch in place, once the fee txs were distributed.

@ljedrz
Copy link
Collaborator

ljedrz commented Feb 12, 2024

Hmm, the way the patch works seems to interfere with the current consensus setup; while I'm pretty sure that we no longer hit the critical speculation bug, it would be best if we could come up with an updated way to reproduce the issue. @raychu86 could you suggest some method that would be the least "intrusive"?

@raychu86
Copy link
Contributor

raychu86 commented Feb 16, 2024

@ljedrz @randomsleep I've created a new proposed fix for this to just do verification for the proposals. Take a look here - #3098. Would be helpful to think about the implications of this fix.

@ozoekwe
Copy link

ozoekwe commented Feb 29, 2024

I tried to reproduce it with the mainnet branch, still not working but will find a way surely

@raychu86
Copy link
Contributor

raychu86 commented Mar 4, 2024

Closed with #3104.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect or unexpected behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants