feezybabee commented 3 months ago

2478590

Summary

Validator node memory leak, attacker could exploit this to halt the network.

Steps To Reproduce

git clone git@github.com:ghostant-1017/mysnarkOS.git && git checkout attack/memory-leak
Start the devnet cd snarkos && ./devnet with 4 validators, 0 clients
Observer the logs, we will find the @@@@[pending] current_items after expire: 28 monotonically increasing

Proof-of-Concept (PoC)

Validators will fetch missing transmissions and insert the callback_sender into pending.
The items in pending will be removed when call remove or clear_expired_callback_for_item. The remove will be called when receiving the response and clear_expired_callback_for_item will NOT clear the expired items because they are different items according to current logic.
And a malicious validator will never respond to any TransmissionRequest.
In even worse case, the malicious validator can send at most MAX_TRANSMISSIONS_PER_BATCH: usize = 50; per propose and 10 proposes per second for example, other validators will soon run out of memory

Supporting Material/References:

Demo: https://github.com/ghostant-1017/mysnarkOS/commit/8b7870b4cfb46befbe856937c5accbb43dfcbe4d#diff-37faac412cc6f27ef4bcf04945c419f8fc1fa2979bba5b4a59272f314562a354

Exploit Demo: https://github.com/ghostant-1017/mysnarkOS/tree/attack/exploit-memory-leak The propose batch rate can go up to about 40,000 proposes per second according to the limit when committee_size = 4

Impact

Validator node memory leak, attacker could exploit this to halt the network.

raychu86 commented 3 months ago

This is a valid but known issue that we already have a pending fix for - https://github.com/AleoHQ/snarkOS/pull/3135.

We just have not gotten around to cleaning it up, burn-in testing the change, and merging it!

ghostant-1017 commented 3 months ago

Thank you for your response! I didn't notice that, feel free to close it!

This is a valid but known issue that we already have a pending fix for - #3135.

We just have not gotten around to cleaning it up, burn-in testing the change, and merging it!

raychu86 commented 2 months ago

We are burn-in testing the change now, but would still value a review from you @ghostant-1017!

Would be helpful to see if your attack is still relevant after this fix.

ghostant-1017 commented 2 months ago

Hi @raychu86 , I've reviewed this pr, and I believe it can prevent this attack.