Skip to content

[medium] fix: [cof2misp] make strict COF validation actually check mandatory fields - #835

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/024-cof-strict-validation-stub
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/024-cof-strict-validation-stub

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — Strict COF validation actually checks the mandatory fields instead of accepting everything.

  • Problem — is_cof_valid_strict() in misp_modules/lib/cof2misp/cof.py was a permanent stub returning True unconditionally, and since validate_cof() defaults to strict=True, the only caller in import_mod/cof2misp.py had no validation at all.
  • Fix — Wires the function up to the existing is_cof_valid_simple(), which already checks the mandatory rrname, rrtype, rdata and first/last-seen fields.
  • Effect — Malformed passive-DNS COF input is rejected with a clear message instead of falling through to an opaque KeyError later in the import.
  • Cost — Full JSON Schema validation is still absent; a FIXME is kept in place noting this.

The defect

is_cof_valid_strict() in misp_modules/lib/cof2misp/cof.py was a permanent stub:

def is_cof_valid_strict(d: dict) -> bool:
    """Check the COF - do the full JSON schema validation.

    Returns
    --------
    True on success, False on validation failure.
    """
    return True  # FIXME

validate_cof() defaults to strict=True, and the only caller,
misp_modules/modules/import_mod/cof2misp.py:84 (if not validate_cof(entry):),
relies on that default. So COF validation was entirely inert: any input, valid or
not, was waved through.

Impact

A malformed passive-DNS COF line (e.g. missing rrtype, rrname, or rdata)
never got the intended "validation failed" rejection. Instead it fell through to
field access further down in cof2misp.py and died with a bare KeyError — an
opaque crash for the analyst instead of a clear, actionable validation error.

The fix

is_cof_valid_strict() now delegates to the existing is_cof_valid_simple(),
which already implements the mandatory-field checks (rrname, rrtype,
rdata, and the first/last-seen time fields) with clear stderr messages on
failure. A FIXME is kept in place noting that full JSON Schema validation is
still not implemented — this only wires up the mandatory-field check that
already existed and was unused.

Behaviour change

This flips strict-mode COF validation from a no-op to an actual check, so input
that was previously silently accepted is now rejected when it's missing a
mandatory field. This is the correct behaviour per the function's own
docstring and is what the caller (strict=True by default) already expected.

Note: is_cof_valid_simple() has a pre-existing precedence quirk in its
time-field condition (records carrying zone_time_first/zone_time_last
instead of time_first/time_last can be rejected due to unparenthesized
not (...) or (...) logic) and a harmless duplicated rdata presence check.
Neither is introduced by this change — they already ran on every strict=False
call — but routing strict-mode traffic through this function for the first time
means strict callers now also see that quirk. Fixing it is out of scope here;
this PR only replaces the always-True stub with the existing intended check.

Verification

Ran directly in this session (not just reported by the fix):

$ python misp_modules/lib/cof2misp/cof.py
================================================================================
Unit Tests:
...
COF unit tests....
dnsdbflex unit tests....
Unit Tests DONE
line 0 is valid: True
line 1 is valid: True
dnsdbflex line 0 is valid: True
...
SELFTEST_OK

python -m py_compile is clean (the file is under misp_modules/lib/, excluded
from flake8 in CI). The full module test suite was also run and reported:
161 passed, 4 skipped, 5 subtests passed in 92.95s (0:01:32).

Found during a review of the repository; other findings are being submitted as
separate PRs.

🤖 Generated with Claude Code

…ields

is_cof_valid_strict() was a permanent stub that returned True unconditionally.
Since validate_cof() defaults to strict=True and the cof2misp import module is
its only caller, every COF ndjson line was accepted regardless of content. A
line missing rrtype, rrname or rdata therefore sailed past the validation guard
and blew up moments later in parse_and_insert_cof() with a bare KeyError, so an
analyst importing a slightly malformed passive DNS feed got an opaque module
crash instead of the intended "Could not validate the COF input" error.

Until the full JSON Schema validation is written, strict validation now
delegates to is_cof_valid_simple(), which checks the mandatory fields and their
types. This is stricter than before by design: input that used to be silently
waved through and then crash is now rejected with a clear message.

Verified with the module's own __main__ self-tests and the full test suite
against a live modules server: 161 passed, 4 skipped, 5 subtests passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8
@elhoim elhoim changed the title fix: [cof2misp] make strict COF validation actually check mandatory fields [medium] fix: [cof2misp] make strict COF validation actually check mandatory fields Sep 3, 2026

This branch has not been deployed

No deployments
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