AVM-1 No Gas Gharges for Failed Transactions

[SA124]

AVM-1 No Gas Gharges for Failed Transactions

Auditor: Movebit
Code: Aptos Core
Severity: Critical
Discovery Methods: Manual Review
Status: Pending
Code Location:
aptos-move/aptos-vm/src/aptos_vm.rs#1654
Descriptions:
Pre-processing checks are performed on transactions as they enter the mempool：

But the SEQUENCE_NUMBER_TOO_NEW error for transactions with sequence_number
greater than the current account.sequence_number is covered in the function. This is as
expected (because future transactions are also going into the pool pending.) But the too_new
error for transactions with sequence_number greater than the current
account.sequence_number is covered in the function. This is as expected (since future
transactions are also going into the transaction pool pending：

At this point the transaction goes to the mempool and is written to the DA. After that it will
read the transaction from DA and execute it when the code is as follows:

At this point, the transaction is still checked for the same things that are checked during
preprocessing.

In this case, the sequence_number is higher than the current sequence_number, resulting in
an error. However, the handling fee is deducted after this check is performed, so no handling
fee is deducted for the transaction. This issue can lead to DA of transactions, mempool
being dosed at 0 cost.
Suggestion:
It is suggested to refer to the design idea of aptos to do the pending processing for the
transaction whose sequence_number is larger than the current sequence_number.

[msjyryxdzzj]

We looked at the logic of the code fix in #542, and it was fixed for the same sequence_number too new, but it could still be attacked in the same way as the previous attack for a different sequence_number too new transaction.

[l-monninger]

@msjyryxdzzj What is your proposed attack? You cannot duplicate sequence numbers will not leave the mempool to the same node under #542?

This also became PR #722 (as it was ultimately signed). See the tests added therein for whether the issue is addressed.

[msjyryxdzzj]

Attacker sends a transaction with seq_number greater than the current seq_number
For example, if the current seq_number is 10, the attacker sends a transaction with a seq_number of 11,12,13,14... different seq_numbers to bypass the repair check.