Page MenuHomePhabricator
Differential D8026

[backport#14696 2/2] New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137.
ClosedPublic
Actions

Authored by PiRK on Oct 21 2020, 14:32.

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC41e566cae16c: [backport#14696 2/2] New regression testing for CVE-2018-17144, CVE-2012-2459…
Summary

CVE-2018-17144 and CVE-2012-2459 are only partially tested for regression.

  • CVE-2018-17144 is not tested for the inflation bug.
  • CVE-2012-2459 is only tested for the mutated block being rejected, not

for the original block being accepted afterwards.

This commit fixes that limitation.

Also added functional test for CVE-2010-5137.

This concludes backport of PR14696 - part 2 of 2
Depends on D8018

Test Plan

ninja && test/functional/test_runner.py p2p_invalid_block.py p2p_invalid_tx.py

Diff Detail

Event Timeline

PiRK created this revision.Oct 21 2020, 14:32
Owners added a reviewer: Restricted Owners Package.Oct 21 2020, 14:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 21 2020, 14:32
PiRK requested review of this revision.Oct 21 2020, 14:32
teamcity edited the summary of this revision. (Show Details)Oct 21 2020, 14:32
PiRK added a comment.Oct 21 2020, 14:35

For the record, I am horrified by how this PR uses a metaclass, type and __subclasses__ to build basic python classes. This is why standard tools cannot detect dead code :)

Harbormaster completed remote builds in B13562: Diff 24894.Oct 21 2020, 15:01
PiRK retitled this revision from New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. to [backport#14696 2/2] New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137..Oct 21 2020, 16:59
jasonbcox requested changes to this revision.Oct 21 2020, 21:53
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/p2p_invalid_block.py
102 ↗(On Diff #24894)

Don't remove the CTOR sorting.

147 ↗(On Diff #24894)

Need to ensure CTOR here too.

This revision now requires changes to proceed.Oct 21 2020, 21:53
PiRK updated this revision to Diff 24924.Oct 22 2020, 06:54

add CTOR sorting to blocks

Harbormaster completed remote builds in B13576: Diff 24924.Oct 22 2020, 07:12
teamcity added a comment.Oct 22 2020, 07:12
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_getchaintips.py                              | ✓ Passed  | 3 s
rpc_help.py                                      | ✓ Passed  | 1 s
rpc_invalidateblock.py                           | ✓ Passed  | 1 s
rpc_misc.py                                      | ✓ Passed  | 1 s
rpc_named_arguments.py                           | ✓ Passed  | 1 s
rpc_net.py                                       | ✓ Passed  | 1 s
rpc_preciousblock.py                             | ✓ Passed  | 1 s
rpc_psbt.py                                      | ✓ Passed  | 22 s
rpc_rawtransaction.py                            | ✓ Passed  | 52 s
rpc_scantxoutset.py                              | ✓ Passed  | 4 s
rpc_setban.py                                    | ✓ Passed  | 2 s
rpc_signmessage.py                               | ✓ Passed  | 1 s
rpc_signrawtransaction.py                        | ✓ Passed  | 1 s
rpc_txoutproof.py                                | ✓ Passed  | 2 s
rpc_uptime.py                                    | ✓ Passed  | 1 s
rpc_users.py                                     | ✓ Passed  | 5 s
rpc_whitelist.py                                 | ✓ Passed  | 1 s
tool_wallet.py                                   | ✓ Passed  | 4 s
wallet_abandonconflict.py                        | ✓ Passed  | 17 s
wallet_address_types.py                          | ✓ Passed  | 15 s
wallet_avoidreuse.py                             | ✓ Passed  | 6 s
wallet_backup.py                                 | ✓ Passed  | 50 s
wallet_balance.py                                | ✓ Passed  | 10 s
wallet_basic.py                                  | ✓ Passed  | 31 s
wallet_coinbase_category.py                      | ✓ Passed  | 1 s
wallet_create_tx.py                              | ✓ Passed  | 6 s
wallet_createwallet.py                           | ✓ Passed  | 2 s
wallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
wallet_disable.py                                | ✓ Passed  | 1 s
wallet_dump.py                                   | ✓ Passed  | 2 s
wallet_encryption.py                             | ✓ Passed  | 5 s
wallet_groups.py                                 | ✓ Passed  | 43 s
wallet_hd.py                                     | ✓ Passed  | 6 s
wallet_import_rescan.py                          | ✓ Passed  | 6 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 2 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 2 s
wallet_labels.py                                 | ✓ Passed  | 1 s
wallet_listreceivedby.py                         | ✓ Passed  | 18 s
wallet_listsinceblock.py                         | ✓ Passed  | 2 s
wallet_listtransactions.py                       | ✓ Passed  | 23 s
wallet_multiwallet.py                            | ✓ Passed  | 13 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 16 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 13 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 4 s

ALL                                              | ✓ Passed  | 830 s (accumulated) 
Runtime: 166 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
deadalnix requested changes to this revision.Oct 22 2020, 12:02
deadalnix added a subscriber: deadalnix.

You need to figure out the CI failure.

These errors messages from CI are absolutely not helpful though.

This revision now requires changes to proceed.Oct 22 2020, 12:02
PiRK added a comment.Oct 22 2020, 14:35
In D8026#188785, @deadalnix wrote:

You need to figure out the CI failure.

These errors messages from CI are absolutely not helpful though.

I'm going to try a rebase. I don't see how a change in a functional test can break a unit test. And this one has given us issues in the past.

PiRK updated this revision to Diff 24949.Oct 22 2020, 14:35

rebase

deadalnix accepted this revision.Oct 22 2020, 14:38
Harbormaster completed remote builds in B13598: Diff 24949.Oct 22 2020, 14:44
jasonbcox requested changes to this revision.Oct 22 2020, 21:16
jasonbcox added inline comments.
test/functional/p2p_invalid_block.py
104 ↗(On Diff #24949)

make_conform_to_ctor should be after this line since you're rehashing one of the txs. The sort order may differ.

This revision now requires changes to proceed.Oct 22 2020, 21:16
PiRK updated this revision to Diff 24990.Oct 23 2020, 07:37

move CTOR sorting after transaction rehash

Harbormaster completed remote builds in B13616: Diff 24990.Oct 23 2020, 07:50
jasonbcox accepted this revision.Oct 23 2020, 17:13
This revision is now accepted and ready to land.Oct 23 2020, 17:13
Closed by commit rABC41e566cae16c: [backport#14696 2/2] New regression testing for CVE-2018-17144, CVE-2012-2459… (authored by lucash-dev <lucash.dev@gmail.com>, committed by PiRK). · Explain WhyOct 24 2020, 13:27
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC41e566cae16c: [backport#14696 2/2] New regression testing for CVE-2018-17144, CVE-2012-2459….

Revision Contents
Changeset List

PathSize
test/
functional/
data/
43 lines
50 lines

Diff 24924

View Options

test/functional/data/invalid_txs.py

Loading...
View Options

test/functional/p2p_invalid_block.py

Loading...