Repository navigation
fix(security): validate CoNLL / CMUdict reader lines to stop malformed-line crash (CWE-20) - #3669
Conversation
Three readers split or index a corpus line without checking its shape, so a
single malformed line raised an uncaught exception out of a standard public
reader method and aborted iteration over the whole corpus (these back the
registered conll2000/conll2002/cmudict corpora):
- conll chunked_words/_sents: `chunk_tag.split("-")` unpack crashed on a tag
with no '-' (and on a chunk type that legitimately contains a '-').
- conll parsed_sents / srl_spans: `tag.split("*")` unpack crashed on a tag
with no '*'.
- cmudict entries/dict/words: `pieces[0]` raised IndexError on a blank line.
Validate each line's shape before unpacking and raise a clear, catchable
ValueError naming the bad tag (matching the reader's existing "Inconsistent
number of columns" error); split chunk tags with maxsplit=1 so hyphenated
chunk types parse; and skip blank CMUdict lines. Well-formed data is
unchanged (CWE-20).
Add nltk/test/unit/test_conll_cmudict_security.py.
db4dc98 to
21aa889
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens NLTK’s CoNLL and CMUdict corpus readers against malformed input lines that previously caused uncaught exceptions during normal iteration, improving robustness and aligning failures with clearer, catchable ValueErrors (CWE-20).
Changes:
- Add explicit shape validation before unpacking CoNLL chunk/parse/SRL tags in
ConllCorpusReader. - Skip blank/whitespace-only lines when reading CMUdict blocks to avoid
IndexError. - Add a new regression test module covering the prior crash cases and behavior preservation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| nltk/corpus/reader/conll.py | Adds guarded parsing for chunk/parse/SRL tags to prevent malformed-tag crashes and provide clearer ValueErrors. |
| nltk/corpus/reader/cmudict.py | Skips blank lines when reading blocks to avoid crashing on whitespace-only lines. |
| nltk/test/unit/test_conll_cmudict_security.py | Adds regression tests for malformed-line handling and preserved parsing behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- conll chunk tags: validate the IOB shape, not just the presence of a "-".
A tag with an unsupported state (e.g. "X-NP") or an empty type (e.g. "B-")
now fails fast with the clear ValueError the message already advertised,
instead of being silently mishandled. The "O" case is still handled before
the split, and hyphenated chunk types (e.g. "B-NP-SBJ") still parse.
- CMUdict regression test: include the counter column ("<word> <counter>
<transcription>") so the data is a well-formed CMUdict line, and assert the
full pronunciation list (the reader keeps pieces[2:]).
ekaf
left a comment
There was a problem hiding this comment.
The Copilot feedback identified two good issues—the need for fuller IOB validation and the need for CMUdict regression data to reflect the real line format—and both appear to be addressed directly in the follow-up. This is a useful merge for NLTK because these readers are standard entry points, and making malformed-line handling clearer and more robust improves real-world library behavior.
Validate CoNLL / CMUdict reader lines to stop malformed-line crash (CWE-20)
Description
Three corpus readers split or index a corpus line without validating its shape,
so a single malformed line raises an uncaught exception out of a standard public
reader method and aborts iteration over the entire corpus. These back the
registered
conll2000,conll2002andcmudictcorpora, so the standard readpath is affected — no unusual interface call is needed.
A tuple-unpack of
str.split(...)raisesValueErrorwhenever the separator isabsent (or appears more than expected), and
pieces[0]raisesIndexErroron ablank line. Confirmed with the report's PoC:
Impact: a single malformed line crashes the worker reading the corpus and
aborts the whole-corpus iteration. The pre-condition is an application reading
user-supplied or third-party data in these formats. No authentication or user
interaction is required. Weakness: CWE-20 (improper input validation).
The fix
Validate each line's shape before unpacking, matching the reader's existing
convention for malformed data (
ConllCorpusReader._read_grid_blockalready doesraise ValueError("Inconsistent number of columns:\n%s" % block)):chunked_words/chunked_sents— split withmaxsplit=1and check theresult.
split("-", 1)additionally keeps chunk types that legitimatelycontain a hyphen (e.g.
B-NP-SBJ), which previously crashed with "too manyvalues to unpack"; a tag that is neither
Onor<state>-<type>now raises aclear, catchable
ValueErrornaming the bad tag:parsed_sentsandsrl_spans/srl_instances— a parse/SRL tag mustcontain exactly one
*word placeholder; otherwise raise a clearValueError(
Malformed parse tag .../Malformed SRL tag ...) instead of the crypticunpack error.
entries/dict/words(CMUdict) — a blank / whitespace-only line carriesno entry, so it is skipped (it is not malformed data — skipping loses
nothing) rather than crashing on
pieces[0]:Properties:
and CMUdict entries parse to exactly the same results as before.
lines that crashed before now read successfully; genuinely malformed tags fail
with a catchable, descriptive
ValueError(consistent with the reader'sexisting "Inconsistent number of columns" error) rather than a cryptic
ValueError: not enough values to unpack/IndexError.Behaviour preservation (verified)
python -m doctest nltk/corpus/reader/conll.pyand.../cmudict.pypass.Tree('NP', [...]); a validSRL grid →
[[[((1, 2), 'V')]]]; CMUdict entries parse unchanged.Testing
nltk/test/unit/test_conll_cmudict_security.py(new, 7 tests): malformedchunk/parse/SRL tags raise
ValueErrorwith a clear message (asserted viapytest.raises(..., match=...)); a hyphenated chunk type now parses; a blankCMUdict line is skipped and both real entries are read; and benign chunk/SRL
parses are preserved. The robustness cases fail against the pre-fix
develop(
not enough values to unpack/too many values to unpack/IndexError)and pass against the fix; the two benign tests pass on both.
black==25.11.0,isort==6.1.0,ruff==0.15.6,pyupgrade --py310-plus(repo-pinned hooks) — clean.