[Script] Fix "split P2CS" vulnerability#2258
Merged
random-zebra merged 4 commits intoPIVX-Project:masterfrom May 8, 2021
Merged
[Script] Fix "split P2CS" vulnerability#2258random-zebra merged 4 commits intoPIVX-Project:masterfrom
random-zebra merged 4 commits intoPIVX-Project:masterfrom
Conversation
random-zebra
force-pushed
the
202103_cs-stricter-checks
branch
2 times, most recently
from
eb6d44b to
14d3956
Compare
Author
|
Rebased on top of #2269 , and added a contextual flag to guard the new rules before v6 activation. |
This was referenced Mar 26, 2021
random-zebra
force-pushed
the
202103_cs-stricter-checks
branch
from
06ac6e2 to
01d4cd5
Compare
Author
|
Rebased on master. Ready for review. |
random-zebra
requested review from
Fuzzbawls and
furszy
and removed request for
Fuzzbawls and
furszy
1 task
random-zebra
force-pushed
the
202103_cs-stricter-checks
branch
from
01d4cd5 to
8298ed5
Compare
The validation for P2CS scriptPubKey is incomplete and doesn't check all the opcodes. This opens a vulnerability. A script could be crafted so that: * it is identified as P2CS (passing IsPayToColdStaking), and recognized by the wallet as ISMINE_SPENDABLE_DELEGATED. * the assumed owner is not actually the owner (or not the only one) of the coins. In this Proof of concept, we craft a script that is recognized as own P2CS by the owner wallet, but can actually be spent with **any** key. This is achieved by including OP_DROP in a strategic position, so that, during the script evaluation, part of the locking condition (included only to fake IsPayToColdStaking) is removed, and replaced by a new condition embedded in the spending scriptSig.
Ensure stack consistency (size, signature and pubkey encoding) during evaluation
check the whole script template (leave only the 20 bytes for the staker keyID and 20 for the owner keyID).
random-zebra
force-pushed
the
202103_cs-stricter-checks
branch
from
8298ed5 to
c8502bf
Compare
furszy
reviewed
May 6, 2021
furszy
approved these changes
May 6, 2021
Fuzzbawls
approved these changes
May 8, 2021
furszy
added a commit
that referenced
this pull request
May 11, 2021
4a5baf3 [Doc] Add cold-staking changes to release notes (random-zebra) c19416f [GUI] Use new opcode for P2CS after v6 enforcement (random-zebra) 45ebfee [RPC] Use new opcode for P2CS after v6 enforcement (random-zebra) 8eefab5 [Tests] Add script test for new opcode (random-zebra) df11631 [Script] Introduce new OP_CHECKCOLDSTAKEVERIFY opcode (random-zebra) b194386 [Refactor] Rename CCSV opcode to OP_CHECKCOLDSTAKEVERIFY_LOF (random-zebra) Pull request description: Extracted from #2267, and rebased on top of #2258. Given the consensus change, introduced in #2274, we can define a more secure `OP_CHECKCOLDSTAKEVERIFY` opcode, which doesn't leave the last output of the coinstake "free" (as we no longer pay masternode/budgets in the coinstake tx). Built on top of: - [x] #2258 - [x] #2274 ACKs for top commit: Fuzzbawls: re-Code ACK 4a5baf3 after rebase, no code changes. furszy: ACK 4a5baf3. Tree-SHA512: ee78b76137e40df05b854f8959755f1b99e7c7328fb13708ba7e3e6dae6357450210ce19ed7a99dc54aa0d6051d0dbd745af70f4b6e9927de5d3cbb2e04fffb6
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Jun 19, 2021
>>> Backports dced6c0 (PIVX-Project#2258) The validation for P2CS scriptPubKey is incomplete and doesn't check all the opcodes. This opens a vulnerability. A script could be crafted so that: * it is identified as P2CS (passing IsPayToColdStaking), and recognized by the wallet as ISMINE_SPENDABLE_DELEGATED. * the assumed owner is not actually the owner (or not the only one) of the coins. In this Proof of concept, we craft a script that is recognized as own P2CS by the owner wallet, but can actually be spent with **any** key. This is achieved by including OP_DROP in a strategic position, so that, during the script evaluation, part of the locking condition (included only to fake IsPayToColdStaking) is removed, and replaced by a new condition embedded in the spending scriptSig.
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Jun 19, 2021
>>> Backports 889a9e7 (PIVX-Project#2258) Ensure stack consistency (size, signature and pubkey encoding) during evaluation
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Jun 19, 2021
>>> Backports 74bc415 (PIVX-Project#2258) check the whole script template (leave only the 20 bytes for the staker keyID and 20 for the owner keyID).
furszy
added a commit
that referenced
this pull request
Jun 20, 2021
62bf095 [Consensus] use v5.2 params to guard new cold-staking rules (random-zebra) 5358c50 [Refactor] Move stack check inside CheckColdStake (random-zebra) 7bec531 [Consensus] Introduce V5.2 network-upgrade params (random-zebra) fecbb4a [Script] Strict test for IsPayToColdStaking (random-zebra) 84ed2d4 [Script] Strict checks for OP_CHECKCOLDSTAKEVERIFY (random-zebra) 9531fb8 [Tests] Proof of concept for P2CS vulnerability (random-zebra) Pull request description: Backports #2258 and #2428 ACKs for top commit: furszy: re code-ACK 62bf095 Fuzzbawls: utACK 62bf095 Tree-SHA512: 97e155f9fd84e8362f1a07ab1dd91216378f451dc3a0e756e543dfa31e37a8d53e3ffd90e2e8d67ee5e4d3a7f0fbcbd6118eaaaea7677c9682cea4c8f5e941bf
random-zebra
added a commit
to random-zebra/PIVX-BlockExplorer
that referenced
this pull request
Jun 21, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is the result of a responsible disclosure for a vulnerability, related to the cold-staking code, made by Flowzzz (of https://keyboardwarrior.be/), on March 15th. After the initial discussion and analysis, along with @furszy and Flowzzz, a critical exploit has been identified and unit-tested.
For this report and collaboration, PIVX has granted the biggest bounty reward of its history (https://twitter.com/_PIVX/status/1372032360636502019).
This PR contains the PoC unit-test, used to reproduce the exploit, and the successive fix.
Problem
The validation for P2CS scriptPubKey is incomplete and doesn't check all the opcodes.
This opens up a possible exploit. A script could be crafted so that:
IsPayToColdStaking), and recognized by the wallet asSPENDABLE_DELEGATEDismine-type.In this Proof of concept, we craft a script that is recognized as own P2CS by the owner wallet, but can actually be spent with any key.
This is achieved by including
OP_DROPin a strategic position, so that, during the script evaluation, part of the locking condition (included only to fake IsPayToColdStaking) is removed, and replaced by a new condition embedded in the spending scriptSig.Fix
IsPayToColdStakingcheck that the script matches exactly the expected template (51 bytes, with 11 fixed bytes, and the remaining 40 for the two keyIDs). This patches the discovered exploit.OP_CHECKCOLDSTAKEVERIFYinEvalScript(at this point of evaluation, the stack must contain only the pubkeyID, the pubkey, and the signature, properly encoded).