Skip to content

fix(finance): validate the ISIN check digit (#440)#473

Open
apoorvdarshan wants to merge 1 commit into
python-validators:masterfrom
apoorvdarshan:fix-440-isin-checksum
Open

fix(finance): validate the ISIN check digit (#440)#473
apoorvdarshan wants to merge 1 commit into
python-validators:masterfrom
apoorvdarshan:fix-440-isin-checksum

Conversation

@apoorvdarshan

Copy link
Copy Markdown

Summary

Fixes #440 (the ISIN portion). validators.isin() accepts any invalid ISIN check digit.

>>> import validators
>>> bool(validators.isin("US0378331005"))   # Apple, genuinely valid
True
>>> bool(validators.isin("US0378331004"))   # same but wrong check digit
True                                          # <-- should be False

Root cause

_isin_checksum() computes a per-character val and doubles it for odd positions, but never accumulates it into check:

check, val = 0, None
for idx in range(12):
    ...
    if idx & 1:
        val += val      # doubles val, but val is then discarded
return (check % 10) == 0  # check is always 0 -> always True

So the checksum is never actually validated — every 12-character string with a two-letter country code and a trailing digit passes. (_cusip_checksum and sedol accumulate correctly and are unaffected.)

Fix

Implement the real ISIN algorithm: expand letters to digits (A=10 … Z=35), keep digits as-is, then run the Luhn checksum over the resulting digit string. Also enforce that the country code (first two chars) is alphabetic and the check digit (last char) numeric, so lowercase/malformed inputs are rejected (as the issue requests).

Verification

  • Validated against python-stdnum's ISIN validator: my implementation agrees on every case tested (real ISINs for Apple/Microsoft/BAE/Alphabet/Bayer/etc. → valid; wrong-check-digit variants → invalid).
  • This cross-check also revealed the existing test fixture JP000K0VF054 was itself an invalid ISIN (correct check digit is 5); it only passed before because the checksum was never computed. Corrected to JP000K0VF055 and added wrong-check-digit fixtures.
  • New tests fail on master and pass with this change. tests/test_finance.py: 31 passed (incl. doctests), up from 21.
  • ruff check and ruff format --check clean on the changed files.
  • The 17 pre-existing tests/crypto_addresses/test_eth_address.py failures are unrelated (they fail identically on master).

Disclosure: prepared with AI assistance; reviewed and cross-checked locally (incl. against python-stdnum).

_isin_checksum() never accumulated into `check`, so it stayed 0 and
`check % 10 == 0` was always true. Any 12-character string with a
two-letter country code and a trailing digit was accepted regardless of
its check digit (e.g. US0378331004, an invalid variant of Apple's
US0378331005, validated as a valid ISIN).

Implement the real algorithm: expand letters to digits (A=10..Z=35),
keep digits as-is, then run the Luhn checksum over the result. Also
enforce that the country code is alphabetic and the check digit numeric,
rejecting lowercase/malformed input as requested in the issue.

Cross-checked against python-stdnum: this also revealed that the existing
'valid' test fixture JP000K0VF054 was itself an invalid ISIN (correct
check digit is 5) that only passed because the checksum was never
computed; corrected to JP000K0VF055 and added wrong-check-digit cases.

Fixes python-validators#440.
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.

Incorrect finance Validation Logic

1 participant