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

PAR-1 DoS Vulnerability Due to Sequencer Not Checking for Sequence Number Too New During Block Packaging #412

Open
SA124 opened this issue Aug 21, 2024 · 8 comments
Assignees
Labels
Source: Audit Issues discovered by audit. suzuka:safety

Comments

@SA124
Copy link

SA124 commented Aug 21, 2024

PAR-1 DoS Vulnerability Due to Sequencer Not Checking for Sequence Number Too New During Block Packaging

Auditor: Movebit
Code: Aptos Core
Severity: Critical
Discovery Methods: Manual Review
Status: Pending
Code Location:
networks/suzuka/suzuka-full-node/src/partial.rs#126,142-227
Descriptions:
The reason for this DoS vulnerability is that the transaction of
sequence_number_too_new will be packaged into the block and the block will be
executed. Attackers can exploit this vulnerability by creating a transaction with a
sequence_number greater than the current value (e.g., sequence_number = 2 when the
current is 0) and setting the
`
expexpiration_timestamp_secs to a large value. After signing
and sending such a transaction, it gets included in a block. Although the transaction itself will
not succeed, repeatedly sending the transaction causes the block to be re-executed multiple
times without incurring any cost to the attacker. Local testing has shown that concurrent
sending of these attack transactions can significantly degrade the node

s performance,
preventing it from handling legitimate transactions.
PoC:

  1. Transfer any amount of tokens to the attacker's address:
    0x60b5f67bb14334b371f207df24d6f717e50941a67f84db471d661de5140e23c9.
  2. Concurrently send the following POST request:
Screenshot 2024-08-21 at 11 03 13 AM

Suggestion:
Transaction Validation: Validate transactions before executing the block to identify and
reject potentially malicious transactions that could lead to DoS attacks.

@mzabaluev mzabaluev added the Source: Audit Issues discovered by audit. label Aug 26, 2024
@l-monninger
Copy link
Collaborator

Although the transaction itself will not succeed, repeatedly sending the transaction causes the block to be re-executed multiple times without incurring any cost to the attacker.

Can you elaborate? Do you mean executed on several replicas or one? Is this simply because of the execute_with_retry or is there something in the VM you're referring to? If the former, the fact that the block retry attempt catches all instead of just block timestamp errors is known and yes needs to be addressed for this.

I don't believe #409 should help with this because there would be only one failure in the attack described.

@jlguochn
Copy link

jlguochn commented Sep 9, 2024

When I send the POST request containing a transaction with an too new sequence_number, the block is executed, and I see log:
execute_block{id-"tDFWeGlcNXIK24EivRC8Vs9UG+VP2Z2ySv8x3bsIjEw="}: suzuka_full_node::partial: Executed block: dec36547
In my local testing setup using Docker Compose, there is only one full node running.

Although the transaction status remains pending, each time I send this request, the node executes the block, consuming computational resources. When these transactions are sent concurrently in high volumes, the node becomes overwhelmed and temporarily unable to respond to other legitimate requests, as the repeated execution of blocks due to the invalid transactions degrades the node’s overall performance.

@l-monninger
Copy link
Collaborator

@jlguochn

That sounds more like general load degradation, perhaps because of the shared channel used when polling for the transaction that is too new. The full node also tends not to run particularly well on the average laptop.

Are you also sending legitimate requests while sending these too new transactions?

@jlguochn
Copy link

I have been sending legitimate requests locally while also performing the DoS. Initially, the legitimate requests were processed normally, but after two minutes of the DoS, they stopped responses.

I have conducted similar tests on my local machine using Aptos Core, and the DoS attack was ineffective due to an idempotent restriction on transactions. https://github.com/aptos-labs/aptos-core/blob/47647ef38f47ab5a3e688d7bc83c477a8c09b77a/mempool/src/core_mempool/transaction_store.rs#L229-L269

Additionally, Aptos does not attempt to package transactions with Sequence_Number_Too_New into blocks. Instead, these are classified as non-ready transactions and tracked using: https://github.com/aptos-labs/aptos-core/blob/47647ef38f47ab5a3e688d7bc83c477a8c09b77a/mempool/src/core_mempool/transaction_store.rs#L73.

This approach effectively prevents such DoS attacks. I hope this information helps.

@l-monninger l-monninger added this to the Centralized Mainnet milestone Sep 12, 2024
@jlguochn
Copy link

jlguochn commented Oct 21, 2024

Another similar and noteworthy attack method involves an attacker constructing a large number of non-ready transactions with different nonce values to try to fill up the mempool.

The Aptos mempool has a memory eviction mechanism to prevent such attacks (link).
When the mempool is full, it refuses to accept non-ready transactions and only accepts ready transactions. It also evicts non-ready transactions from the mempool to ensure that ready transactions are successfully received.

However, the current check_is_full_after_eviction function has some imperfections, in certain situations, the mempool that allowed its parking lot to be filled, limiting a node’s ability to accept new transactions.

I have reported this issue to Aptos, and they have fixed it in version v1.19.1. The related fix commit is here: aptos-labs/aptos-core@70806ff.

@l-monninger
Copy link
Collaborator

l-monninger commented Oct 24, 2024

These should be fixed in Aptos Mempool and our mempool by #722 , #628 , I believe.

@jlguochn
Copy link

The recent fix appears to be effective.

I would like to recommend further enhancements to the transaction eviction mechanism within the Aptos Core, particularly concerning the specific commit link.

An adversary could potentially set the expiration_timestamp_secs to a large value, effectively bypassing self.core_mempool.gc(). Additionally, an attacker could send multiple transactions with sequentially incrementing sequence_number values, which the mempool would receive and maintain as pending. Under these circumstances, the mempool could still become saturated in a DoS scenario. And the check_is_full_after_eviction function will ensure that, even when the mempool reaches its full capacity, ready transactions are prioritized for processing.

@jlguochn
Copy link

The current Aptos mempool eviction mechanism is insufficient and poses a potential DoS risk. I recommend referring to PR aptos-labs/aptos-core#14586 for improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Source: Audit Issues discovered by audit. suzuka:safety
Projects
None yet
Development

No branches or pull requests

4 participants