Skip to content

descriptor: validate public keys when parsing - #859

Open
fametrano wants to merge 1 commit into
bitcoin-core:masterfrom
fametrano:btclib-666-validate-pubkey
Open

fametrano wants to merge 1 commit into
bitcoin-core:masterfrom
fametrano:btclib-666-validate-pubkey

Conversation

@fametrano

@fametrano fametrano commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

A raw-hex public key in a descriptor is never checked to be a valid point. So pkh(020000000000000000000000000000000000000000000000000000000000000005) parses, although no curve point has that x-coordinate. Bitcoin Core rejects an invalid key at parse time.

This PR checks a raw-hex key with the same rules as Core's ParsePubkeyInner. What is accepted depends on where the key appears:

  • inside tr(): a 32-byte x-only key or a 33-byte compressed key;
  • elsewhere: a 33-byte compressed key, and a 65-byte uncompressed key only at the top level or under sh() (rejected under wpkh()/wsh(), per BIP141);
  • hybrid (0x06/0x07) and off-curve or wrong-length keys are rejected everywhere.

hwilib/key.py gains two small on-curve checks, point_on_curve and x_coord_on_curve. The existing point helpers recover a coordinate without checking the curve equation. Extended keys (xpubs) are not affected. Tests cover the accepted and rejected cases.

The same change for Electrum's descriptor parser, which shares this code, is spesmilo/electrum#10951.

Made with my usual tools: a computer, the Internet and an LLM. The mistakes, as usual, are all mine.

@fametrano

Copy link
Copy Markdown
Contributor Author

The one red check on this PR is test-bitbox01 / Python 3.12 bitbox01 cli, and it isn't caused by this change: the job failed before any test ran, because bitcoind never started:

Error: Unable to bind to 0.0.0.0:44435 on this computer. Bitcoin Core is probably already running.
Error: Failed to listen on any port. Use -listen=0 if you want this.
Traceback (most recent call last):
  File "/__w/HWI/HWI/test/./run_tests.py", line 124, in <module>
    bitcoind = Bitcoind.create(args.bitcoind)
RuntimeError: bitcoind failed with exit code 1

That looks like a port collision starting the daemon, not a descriptor-parsing failure. There's a single run on this head (b5b8e25c, run 34319824959, attempt 1), and it has never been retried, so the red has been sitting there since September 9, 2026. #857 and #858, which touch the same file, are green on all 294 jobs.

Could someone re-run that job, and take a look at the change itself? The branch is even with master, so there's no rebase that would produce a fresh run, and I can't trigger one myself.

@fametrano
fametrano force-pushed the btclib-666-validate-pubkey branch from b5b8e25 to 43a0ba3 Compare September 23, 2026 21:14
A public key written as raw hex in a descriptor is checked against the
secp256k1 curve for the context it appears in. A 33-byte compressed key
is accepted everywhere; a 32-byte x-only key only inside tr(); a 65-byte
uncompressed key only at the top level and under sh(), and never under
the segwit contexts wpkh() and wsh() or inside tr(). A key whose length,
prefix, or encoded point is invalid is rejected with a ValueError.

Previously such keys were only tested for being hex rather than an
extended key, so a descriptor carrying off-curve, wrong-length,
bad-prefix, or context-inappropriate bytes parsed without complaint.
@fametrano
fametrano force-pushed the btclib-666-validate-pubkey branch from 43a0ba3 to 165d43a Compare September 29, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant