feat: add BitcoinJ to extkey deserialization target by kuliq23 · Pull Request #396 · bitcoinfuzz/bitcoinfuzz

kuliq23 · 2026-01-21T15:56:33Z

Due to how deserialization works in BitcoinJ, (parent key is needed to get a valid childnumber) child=00000000; is always present in the final output. This makes fuzzing fail.

see the "zeroed" child e.g. here:

xprv9s2SHxhPMGLurjhydgXZ33GXgYeay6pqsmiAdzuYV8LPqXwxtMogrAxqoe5xyjkjmjss9DADrpx7Qd8PPzcxry6GPv4uks6qustFDtx3o1F

Module: NBitcoin
Result: depth=00;fp=01000000;child=00000004;chaincode=0000000000000000000000000000000000000000000000000000000000000000;key=0000000000000000000000004a00000000000000000000000000000000000000
Module: BitcoinJ
Result: depth=00;fp=01000000;child=00000000;chaincode=0000000000000000000000000000000000000000000000000000000000000000;key=0000000000000000000000004a00000000000000000000000000000000000000

or here

tprv8ZgxMBitZRetiHuCVmbHHrYKs1sXtrcZ2ZyL2MfemneaEck838icS6QjaSjG9Y35Nrg47FXHtAwPjHv6ogvG3d9cB3XWmwCxSYaZ3xhQq5Q

Module: NBitcoin
Result: depth=00;fp=00000000;child=f7ffffff;chaincode=ffffffff00008200000000000000000000000000000000000000000000000000;key=000000000000000000000000000000000000000a000000000000000000000000
Module: BitcoinJ
Result: depth=00;fp=00000000;child=00000000;chaincode=ffffffff00008200000000000000000000000000000000000000000000000000;key=000000000000000000000000000000000000000a000000000000000000000000

Due to the nature of fuzzing itself, it is impossible to provide a valid parent key for every generated extkey.

I see 3 options we could go about this:

Leave out BitcoinJ from this target all together
Make an exception in output comparison when BitcoinJ is used
Theoretically, a parent child duo could be generated using deriveChildKey but I find it out of the scope of this target, or?

Let me know your thoughts.

erickcestari

Thanks!

BitcoinJ will only zero the child number when it receives a root key (depth = 0). However, in those cases, we can extract the child number from the base58 string ourselves, so we can avoid the fuzzer crashing due to false positives.

erickcestari · 2026-01-22T18:08:08Z

BitcoinJ appears to have the same issue as NBitcoin: it misses test vector 5, and fails on some of them.

https://github.com/bitcoinj/bitcoinj/blob/master/core/src/test/java/org/bitcoinj/crypto/BIP32Test.java#L37

test vector: xprv9s21ZrT9nLigZg99v6iAHBtnXvm4XZPw6FuFn9ccQsFC29kkYQaURG1TwP58GadyYjt6xDPeNTQzDgcEwGdvTtcJtotWEAcBEPByoix8aWR

Module: Rustbitcoin
Result: INVALID (Xpriv error: non-zero parent fingerprint in master key; Xpub error: non-zero parent fingerprint in master key)
Module: BitcoinJ
Result: depth=00;fp=0000000a;child=00000000;chaincode=0000000000000000000000000000000000000000000000000000000000000000;key=0000000000000001000000000000000000000000000000000000000000000000

Nice work!

Do you want to open a PR/issue reporting it?

kuliq23 · 2026-01-22T19:48:48Z

I can create an Issue now, but I won't get to the PC until Wednesday. You can create the PR and link the Issue if you want.

As for the comments, I will incorporate them next week.
Thank you for understanding

erickcestari · 2026-01-29T12:38:46Z

@kuliq23 Let me know when this PR is ready for review. Thanks!

kuliq23 · 2026-01-29T18:19:13Z

@kuliq23 Let me know when this PR is ready for review. Thanks!

Ready, thanks for all the comments.
As you may noticed, I tend to apply the suggested changes manually instead of using GitHub’s commit suggestion feature, as it can complicate history during rebases. Hope that’s okay with you.

erickcestari · 2026-01-30T14:02:17Z

@kuliq23 Let me know when this PR is ready for review. Thanks!

Ready, thanks for all the comments. As you may noticed, I tend to apply the suggested changes manually instead of using GitHub’s commit suggestion feature, as it can complicate history during rebases. Hope that’s okay with you.

It's fine to apply those manually. I actually prefer this way.

erickcestari

It's working, but I found one edge case in the way it sends input from C++ code to Java. It removes everything from the NUL byte onwards, resulting in BitcoinJ receiving only part of the input, which will cause the fuzzer to crash later when BitcoinJ parses it and accept as valid.

It's better to send a byte array to the JVM, which interprets the bytes as a string. This makes the decodeBase58 in BitcoinJ receive the same string/bytes as other implementations.

example of input that would make the fuzzer crash:

xxd -p crash-a4c9dea519d7fafdbcd5f6b0b067d8084c354cc4

7470756245677762737772566637677234724457396e39624e5554377544
37536f6a366a385879745a3771537742416932613934444a796868655672
425558386550325a6d644c775875517350694c41656334344b76534d7232
516f676955477a6b355772717858466a687a356f51000000000000000000
000000000000000000000000000000000000000000000000000000000000
000000000000000000000000000000000000000000000000000000000000
00000000000000000000000000000000

kuliq23 · 2026-02-02T12:06:46Z

It's working, but I found one edge case in the way it sends input from C++ code to Java. It removes everything from the NUL byte onwards, resulting in BitcoinJ receiving only part of the input, which ...

Very good catch, thank you!!

EDIT: It seems like I updated submodules by accident, will fix this...

After these changes, just out of curiosity, I tried comparing only NBitcoin and BitcoinJ - BitcoinJ does NOT like when keybytes are 0x0...001 ?

try e.g. these keys:
xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21TeqtDeZVxb
xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo37H4B5uiRXehnYNiCxYXwAB7aUy3GPVrfB7ovuWy8CqZrPjymmga6bALUaxJGJg66Xsn
tprv8ZgxMCzDvi63V6VZQqkXooMk2DcE1nVJtGeE6RNtaFrRRmS1q2EcgiYUsJZ9WJUK5KDn3sKK12uj1sjfLA2CvATLSWW2TD66RsXk7wzdhko

erickcestari

You have accidentally downgraded the eclair and 'core lightning submodules.

erickcestari · 2026-02-02T13:34:03Z

After these changes, just out of curiosity, I tried comparing only NBitcoin and BitcoinJ - BitcoinJ does NOT like when keybytes are 0x0...001 ?

try e.g. these keys: xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21TeqtDeZVxb xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo37H4B5uiRXehnYNiCxYXwAB7aUy3GPVrfB7ovuWy8CqZrPjymmga6bALUaxJGJg66Xsn tprv8ZgxMCzDvi63V6VZQqkXooMk2DcE1nVJtGeE6RNtaFrRRmS1q2EcgiYUsJZ9WJUK5KDn3sKK12uj1sjfLA2CvATLSWW2TD66RsXk7wzdhko

Interesting. They have a comment that explains the rationale. So, we should probably skip those cases.

https://github.com/bitcoinj/bitcoinj/blob/b2d8af7aad0baff7a4dc2fb9bf67648805327ce7/core/src/main/java/org/bitcoinj/crypto/ECKey.java#L227-L238

bitcoinj skip when privkeybytes == (0 || 1)

kuliq23 · 2026-02-02T17:06:44Z

Interesting. They have a comment that explains the rationale. So, we should probably skip those cases.

https://github.com/bitcoinj/bitcoinj/blob/b2d8af7aad0baff7a4dc2fb9bf67648805327ce7/core/src/main/java/org/bitcoinj/crypto/ECKey.java#L227-L238

Fixed the submodules and added a skip for those cases. I suppose checks fail due to previously discussed issue with not-defined CXX in the CI runner.

erickcestari · 2026-02-02T18:21:33Z

Fixed the submodules and added a skip for those cases. I suppose checks fail due to previously discussed issue with not-defined CXX in the CI runner.

Yep, I will hardcode the CXX flags to prevent this error from happening again.

erickcestari

Thanks! Nice work!

ACK 6ce3b41

erickcestari self-requested a review January 22, 2026 13:45

erickcestari requested changes Jan 22, 2026

View reviewed changes

Comment thread modules/bitcoinj/app/src/main/kotlin/wrapper/Wrapper.kt Outdated

Comment thread modules/bitcoinj/app/src/main/kotlin/wrapper/Wrapper.kt

Comment thread modules/bitcoinj/module.cpp Outdated

Comment thread modules/bitcoinj/app/src/main/kotlin/wrapper/Wrapper.kt Outdated

kuliq23 mentioned this pull request Jan 22, 2026

Missing BIP-32 Test vector 5 validation bitcoinj/bitcoinj#4054

Open

kuliq23 force-pushed the bip32-bitcoinj-add-deser branch from dc9442d to fbc0dd8 Compare January 28, 2026 15:04

kuliq23 marked this pull request as ready for review January 29, 2026 18:12

erickcestari self-requested a review January 29, 2026 21:33

erickcestari requested changes Jan 30, 2026

View reviewed changes

Comment thread modules/bitcoinj/module.cpp Outdated

Comment thread modules/bitcoinj/module.cpp Outdated

Comment thread modules/bitcoinj/app/src/main/kotlin/wrapper/Wrapper.kt Outdated

kuliq23 force-pushed the bip32-bitcoinj-add-deser branch 2 times, most recently from 6007ae6 to c04d373 Compare February 2, 2026 11:58

erickcestari requested changes Feb 2, 2026

View reviewed changes

kuliq23 force-pushed the bip32-bitcoinj-add-deser branch 2 times, most recently from 868ac40 to 10fe52b Compare February 2, 2026 14:20

feat: Add BitcoinJ to deserialize extkey target

6ce3b41

bitcoinj skip when privkeybytes == (0 || 1)

kuliq23 force-pushed the bip32-bitcoinj-add-deser branch from 10fe52b to 6ce3b41 Compare February 2, 2026 15:23

erickcestari approved these changes Feb 2, 2026

View reviewed changes

erickcestari merged commit 22b9296 into bitcoinfuzz:v2 Feb 2, 2026
34 of 51 checks passed

erickcestari mentioned this pull request Feb 2, 2026

fix(nbitcoin): deserialize all bytes on extended_key target #416

Merged

Uh oh!

Conversation

kuliq23 commented Jan 21, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erickcestari commented Jan 22, 2026

Uh oh!

kuliq23 commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erickcestari commented Jan 29, 2026

Uh oh!

kuliq23 commented Jan 29, 2026

Uh oh!

erickcestari commented Jan 30, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kuliq23 commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

erickcestari commented Feb 2, 2026

Uh oh!

kuliq23 commented Feb 2, 2026

Uh oh!

erickcestari commented Feb 2, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kuliq23 commented Jan 22, 2026 •

edited

Loading

kuliq23 commented Feb 2, 2026 •

edited

Loading