[bitcoin-dev] Full Disclosure: CVE-2021-31876 Defect in Bitcoin Core's bip125 logic

From: Antoine Riard <antoine.riard@gmail•com>
To: Bitcoin Protocol Discussion <bitcoin-dev@lists•linuxfoundation.org>
Subject: [bitcoin-dev] Full Disclosure: CVE-2021-31876 Defect in Bitcoin Core's bip125 logic
Date: Thu, 6 May 2021 09:55:53 -0400	[thread overview]
Message-ID: <CALZpt+GK4WNBmKim3w9LAd1b69+uAyAsNu5tVniHzN6Ue4KJCw@mail.gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 7675 bytes --]

Hi,

I'm writing to report a defect in Bitcoin Core bip125 logic with minor
security and operational implications for downstream projects. Though this
defect grieves Bitcoin Core nodes 0.12.0 and above, base layer safety isn't
impacted.

# Problem

Bip 125 specification describes the following signalling mechanism :

"
This policy specifies two ways a transaction can signal that it is
replaceable.

* Explicit signaling: A transaction is considered to have opted in to
allowing replacement of itself if any of its inputs have an nSequence
number less than (0xffffffff - 1).

* Inherited signaling: Transactions that don't explicitly signal
replaceability are replaceable under this policy for as long as any one of
their ancestors signals replaceability and remains unconfirmed.

One or more transactions currently in the mempool (original transactions)
will be replaced by a new transaction (replacement transaction) that spends
one or more of the same inputs if,

# The original transactions signal replaceability explicitly or through
inheritance as described in the above Summary section.
"

An unconfirmed child transaction with nSequence = 0xff_ff_ff_ff spending an
unconfirmed parent with nSequence <= 0xff_ff_ff_fd should be replaceable as
the child transaction signals "through inheritance". However, the
replacement code as implemented in Core's `PreChecks()` shows that this
behavior isn't  enforced and Core's mempool rejects replacement attempts of
an unconfirmed child transaction.

Branch asserting the behavior is here :
https://github.com/ariard/bitcoin/commits/2021-03-test-rbf

# Solution

The defect has not been patched.

# Downstream Projects Affected

* LN : State-of-the-art pinning attacks against second-stage HTLCs
transactions were thought to be only possible by exploiting RBF rule 3 on
the necessity of a higher absolute fee [0]. However, this replacement
defect opens the way for an attacker to only pin with an opt-out child
without a higher fee than the honest competing transaction. This lowers the
cost of attack as the malicious pinning transaction only has to be above
mempools'min feerate. This also increases odds of attack success for a
reduced feerate diminishes odds of confirmation ending the pinning.

A functional test demo illustrating cases is available on this branch:
https://github.com/ariard/bitcoin/commits/2021-05-htlc-preimage-pinnings

LN nodes operators concerned by this defect might favor anchor outputs
channels, fully mitigating this specific pinning vector.

* Onchain DLC/Coinswap/Vault : Those contract protocols have also multiple
stages of execution with time-sensitive transactions opening the way to
pinning attacks. Those protocols being non-deployed or in early phase, I
would recommend that any in-protocol competing transactions explicitly
signal RBF.

* Coinjoin/Cut-Through : if CPFP is employed as a fee-bumping strategy, if
the coinjoin transaction is still laying in network mempools, if a
fee-bumping output is spendable by any protocol participant, this
fee-bumping mechanism might be halted by a malicious protocol participant
broadcasting an low-feerate opt-out child. According to bip125, if the
coinjoin parent tx signals replaceability, the child transaction should be
replaceable, whatever its signaling. However Core doesn't apply this
policy. RBF of the coinjoin transaction itself should be used as a
fallback. I'm not aware of any deployed coinjoin using such
"anyone-can-bump" fee-bumping strategy.

* Simple wallets : RBF engines' behaviors might be altered in ways not
matching the intent of their developers. I invite RBF engines dev to verify
what those components are doing in the light of disclosed information.

# Discovery

While reviewing the LN dual-funding flow, I inquired on potential new DoS
vectors introduced by relying on counterparty utxos in this following
analysis [1]. The second DoS issue "RBF opt-out by a Counterparty
Double-Spend" is relying on a malicious chain of transactions where the
parent is signaling RBF opt-in through nSequence<=0xff_ff_ff_ff-1 but the
child, servicing as a pinning transaction, opt-out from the RBF policy.
This pinning trick conception was matching my understanding of Core code
but while reading again the specification, I observed that it was
inconsistent from the inherited signaling mechanism as described in the
bip's "Summary" section.

After exercising the logic, I did submit the defect to Dave Harding, asking
confirmation of divergence between Bitcoin Core and BIP 125. Soon after, he
did confirm it and pointed that the defect has been there since the 2015's
PR introducing the opt-in RBF, advicing to to consider security
implications for deployed second-layer protocols. After noticing the minor
implications for pinning attacks on second-stage LN transactions while
talking with Matt Corallo, I did disclose to the Bitcoin Core security list.

My initial report was recommending avoiding a covert patch in the mempool
as risks of introducing DoS in this part of the codebase seemed to outweigh
security of deployed LN channels. This direction was agreed by the opinions
expressed on the security list. Beyond, there was a lack of agreement on
how to proceed with the disclosure as so far in the history project,
transaction relay policy have not been considered as strongly reliable.
Though from now on, L2 protocols like Lightning are making assumptions on
subset of this policy for their safety, such as the highlighted RBF one.

Defect was disclosed to the LN projects maintainers, informing them that
currently in deployment anchor outputs protocol upgrade was mitigating
against this defect though old channels will stay vulnerable. To the best
of my knowledge, I didn't identify other deployed protocols of which coins
safety are impacted by this defect.

# Ecosystem Observations

This long-standing defect with benign security implications provided an
opportunity to exercise coordinated security disclosure across layers and
development teams.

IMO, it underlies few interesting points:
* the lack of an established policy for coordinated security disclosures
between a base layer implementation and its downstream projects
* the lack of a clear methodology to identify downstream projects affected
by a transaction relay policy wreckage
* the lack of minimally-disruptive, emergency upgrade mechanisms
implemented by downstream projects [2]

Finally, security implications for downstream projects provoked by base
layer issues shouldn't be minimized as they do have a risk of windblow on
base layer operations. I believe we should minimize risks of disaster
scenarios such as thousands of LN channels manually closed by worried
operators due to a non-concerted security disclosure, provoking mempool
cloaks and disrupting regular transactions for a while.

# Timeline

2021-03-18 : Defect discovered, report to Dave Harding original author of
bip125, confirmation of the defect
2021-03-19 : Disclosure to the Bitcoin Core security list, Dave Harding,
Matt Corallo, acknowledgment of the issue
2021-04-05 : Disclosure to the LN projects maintainers (c-lightning, lnd,
eclair, electrum, rust-lightning)
2021-04-28 : CVE-2021-31876 assigned
2021-05-06 : Full disclosure to the bitcoin-dev mailing list

I believe the information reported is correct and reflects the best of my
knowledge, please point any shortcoming.

Cheers,
Antoine

[0]
https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-April/002639.html
[1] See "On Mempool Funny Games against Multi-Party Funded Transactions",
published 2021-05-06
[2] Such as
https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-July/002763.html

[-- Attachment #2: Type: text/html, Size: 8442 bytes --]