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 https://github.com/movementlabsxyz/movement/pull/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.