Skip to content

fix(crypto_addresses): correct testnet segwit prefix and reject non-hex ETH addresses#466

Open
Jordan-Bourillot wants to merge 1 commit into
python-validators:masterfrom
Jordan-Bourillot:fix/crypto-address-validators
Open

fix(crypto_addresses): correct testnet segwit prefix and reject non-hex ETH addresses#466
Jordan-Bourillot wants to merge 1 commit into
python-validators:masterfrom
Jordan-Bourillot:fix/crypto-address-validators

Conversation

@Jordan-Bourillot

Copy link
Copy Markdown

This fixes two correctness bugs in the crypto_addresses validators. Both are covered by new regression tests, and the full test suite still passes.

1. btc_address rejects valid testnet segwit addresses

The segwit branch is selected when the value starts with bc or tb, but the regexp only matched bc or tc:

re.compile(r"^(bc|tc)[0-3][02-9ac-hj-np-z]{14,74}$").match(value)
if value[:2] in ("bc", "tb")
else _validate_old_btc_address(value)

Because the gate accepts tb while the pattern expects tc, every valid testnet bech32 address is rejected:

btc_address("tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx")  # ValidationError, should be valid

tc is not a valid Bitcoin HRP and is unreachable through the value[:2] in ("bc", "tb") gate, so it is a typo for tb. Fix: (bc|tc)(bc|tb). Mainnet bc1... is unaffected.

2. eth_address accepts non-hex "checksum" addresses

_validate_eth_checksum_address checked only the length of the stripped address, not that its characters were hexadecimal. The EIP-55 loop only constrains the case of letters, so caseless / non-hex characters pass it vacuously:

eth_address("0x" + "*" * 40)  # True, should be invalid

Fix: require the stripped address to be exactly 40 hex digits before running the checksum (this also subsumes the previous length check).

Tests

  • test_btc_address: added two valid testnet bech32 addresses (BIP173 vectors) to the valid fixtures.
  • test_eth_address: added a non-hex input to the invalid fixtures.

Both new cases fail on master and pass with this change. ruff reports no issues and the full suite (898 passed) is green.

…ex ETH

Two correctness bugs in the crypto address validators:

btc_address: the segwit branch is taken when the value starts with "bc"
or "tb", but the regexp only matched "bc" or "tc". Every valid testnet
bech32 address (tb1...) was therefore rejected. Correct the prefix to "tb".

eth_address: _validate_eth_checksum_address only checked the length of
the stripped address, not that its characters were hexadecimal. Inputs
made of non-hex, caseless characters (e.g. "0x" + "*" * 40) passed the
checksum loop vacuously and were accepted as valid. Require 40 hex
digits before running the checksum.

Existing fixtures are unchanged; added testnet bech32 addresses and a
non-hex input as regression tests.
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