Skip to content

bech32: add BIP-173 padding validation #8419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erickcestari
Copy link
Contributor

@brunoerg has been doing differential fuzzing on CLightning and discovered an offer that crashes our differential fuzzing target in bitcoinfuzz. I found that CLightning accepts malformed offers containing invalid bech32 data that violates the BIP-173 specification, creating interoperability issues with other Lightning implementations (rust-lightning, lightning-kmp) that correctly reject such data.

BIP-173 says:

Re-arrange those bits into groups of 8 bits. Any incomplete group at the end MUST be 4 bits or less, MUST be all zeroes, and is discarded.

offer:

lno1qgqqvqtezcss8kkkkkkkkkk3srkklllllllvw03s33s333p444444444444pgqpq8

lightning-kmp check: https://github.com/ACINQ/bitcoin-kmp/blob/master/src/commonMain/kotlin/fr/acinq/bitcoin/Bech32.kt#L206

rust-lightning check: https://github.com/rust-bitcoin/rust-bech32/blob/master/src/primitives/iter.rs#L180

BOLT12 is missing a test case like this. Perhaps this could be a useful new test vector?

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

Changelog-Fixed: Reject bech32 data with >4 bits of padding or non-zero padding
bits, per BIP-173 specification.
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