Skip to content

fix(security): validate CoNLL / CMUdict reader lines to stop malformed-line crash (CWE-20) - #3669

Merged
ekaf merged 2 commits into
nltk:developfrom
LinZiyuu:fix/conll-cmudict-crash
Jul 1, 2026
Merged

ekaf merged 2 commits into
nltk:developfrom
LinZiyuu:fix/conll-cmudict-crash

Conversation

@LinZiyuu

Copy link
Copy Markdown
Contributor

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, conll2002 and cmudict corpora, so the standard read
path is affected — no unusual interface call is needed.

# nltk/corpus/reader/conll.py
(state, chunk_type) = chunk_tag.split("-")   # L276 chunked_words/_sents
(left, right)       = parse_tag.split("*")   # L313 parsed_sents
(left, right)       = srl_tag.split("*")     # L354 srl_spans / srl_instances

# nltk/corpus/reader/cmudict.py
pieces = line.split()
entries.append((pieces[0].lower(), pieces[2:]))   # L87 entries/dict/words

A tuple-unpack of str.split(...) raises ValueError whenever the separator is
absent (or appears more than expected), and pieces[0] raises IndexError on a
blank line. Confirmed with the report's PoC:

chunked_words()  on "cat NN BADCHUNK"  -> ValueError: not enough values to unpack
parsed_sents()   on "the DT NOSTAR"    -> ValueError: not enough values to unpack
entries()        on a "   " blank line -> IndexError: list index out of range

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_block already does
raise ValueError("Inconsistent number of columns:\n%s" % block)):

  • chunked_words/chunked_sents — split with maxsplit=1 and check the
    result. split("-", 1) additionally keeps chunk types that legitimately
    contain a hyphen
    (e.g. B-NP-SBJ), which previously crashed with "too many
    values to unpack"; a tag that is neither O nor <state>-<type> now raises a
    clear, catchable ValueError naming the bad tag:

    parts = chunk_tag.split("-", 1)
    if len(parts) != 2:
        raise ValueError(
            f"Malformed chunk tag {chunk_tag!r}: expected 'O' or "
            "'<state>-<type>' (e.g. 'B-NP')"
        )
    (state, chunk_type) = parts
  • parsed_sents and srl_spans/srl_instances — a parse/SRL tag must
    contain exactly one * word placeholder; otherwise raise a clear ValueError
    (Malformed parse tag ... / Malformed SRL tag ...) instead of the cryptic
    unpack error.

  • entries/dict/words (CMUdict) — a blank / whitespace-only line carries
    no entry, so it is skipped (it is not malformed data — skipping loses
    nothing) rather than crashing on pieces[0]:

    pieces = line.split()
    if not pieces:
        continue
    entries.append((pieces[0].lower(), pieces[2:]))

Properties:

  • No behaviour change for well-formed data. Ordinary chunk/parse/SRL grids
    and CMUdict entries parse to exactly the same results as before.
  • More robust, not just clearer. Hyphenated chunk types and blank CMUdict
    lines that crashed before now read successfully; genuinely malformed tags fail
    with a catchable, descriptive ValueError (consistent with the reader's
    existing "Inconsistent number of columns" error) rather than a cryptic
    ValueError: not enough values to unpack / IndexError.
  • Minimal & local. Four guarded unpacks; no public signature change.

Behaviour preservation (verified)

  • python -m doctest nltk/corpus/reader/conll.py and .../cmudict.py pass.
  • Valid parses are identical: a benign chunk grid → Tree('NP', [...]); a valid
    SRL grid → [[[((1, 2), 'V')]]]; CMUdict entries parse unchanged.

Testing

  • nltk/test/unit/test_conll_cmudict_security.py (new, 7 tests): malformed
    chunk/parse/SRL tags raise ValueError with a clear message (asserted via
    pytest.raises(..., match=...)); a hyphenated chunk type now parses; a blank
    CMUdict 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.

@LinZiyuu LinZiyuu changed the title Validate CoNLL / CMUdict reader lines to stop malformed-line crash (CWE-20) fix(security): validate CoNLL / CMUdict reader lines to stop malformed-line crash (CWE-20) Jun 25, 2026
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.
@LinZiyuu
LinZiyuu force-pushed the fix/conll-cmudict-crash branch from db4dc98 to 21aa889 Compare June 26, 2026 01:21
@ekaf
ekaf requested a review from Copilot June 26, 2026 09:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread nltk/corpus/reader/conll.py Resolved
- 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 ekaf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ekaf
ekaf merged commit de9bcb2 into nltk:develop Jul 1, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants