Hi Antoine,

Excellent work as usual!

When this was initially reported, I suspected that btcd wasn't actually
affected by this issue (proper RBF inheritance). I wrote a unit test earlier
today to confirm this: https://github.com/btcsuite/btcd/pull/1719.

I'm particularly fond of btcd's implementation of RBF signalling:
https://github.com/btcsuite/btcd/blob/master/mempool/mempool.go#L603.

The separation of concerns here makes it (IMO) much easier to follow (as
well as uint test) compared to bitcoind's `MemPoolAccept::PreChecks`
function. Its recursive nature makes the inherited replaceability explicit.

-- Laolu


On Thu, May 6, 2021 at 8:49 AM Antoine Riard via bitcoin-dev <
bitcoin-dev@lists.linuxfoundation.org> wrote:

> 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
> _______________________________________________
> bitcoin-dev mailing list
> bitcoin-dev@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
>