Skip to content

fix: [openioc] Do not set in_content for self-closed <Content/> - #29

Draft
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/openioc-empty
Draft

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/openioc-empty

Conversation

@elhoim

@elhoim elhoim commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — Stop a self-closed <Content/> from leaving in_content set and fabricating a bogus IOC attribute.

  • Problem — In load_openioc, the Content arm matched Event::Start(e) | Event::Empty(e) and always set in_content = true, but quick-xml emits no Event::End for a self-closed element, so the flag stuck.
  • Fix — The matched event is now bound and in_content is set only when it is an Event::Start; a self-closed Content element is a no-op, as it carries no text.
  • Effect — A stray or whitespace Text node after <Content/> is no longer attributed to the stale current_search, so imports stop inventing attributes.
  • CI — Red for repo-level reasons unrelated to this PR: the job ran on ubuntu-22.04 (no php8.3 mod_php) and test_event_crud_lifecycle raced the publish worker; both fixed in PR fix: [ci] Run integration tests on ubuntu-24.04 to match php8.3 #53.

Finding

self-closed leaves in_content stuck true
The Content arm fires for Event::Empty too, so there is no matching End to clear the flag; the next unrelated Text node is attributed to the stale current_search, fabricating a bogus IOC.

Change

The Content-handling arm in load_openioc matched Event::Start(e) | Event::Empty(e) and unconditionally set in_content = true. quick-xml never emits a corresponding Event::End for a self-closed element (Event::Empty), so a self-closed left in_content stuck true. The next Text node parsed anywhere afterward (whitespace, stray text, or a subsequent unrelated element's text) got attributed to the still-set current_search, fabricating a bogus MISP attribute. Fixed by binding the matched event with ref ev @ (...) and only setting in_content=true when matches!(ev, Event::Start(_)) is true, leaving Event::Empty Content as a no-op (correct, since it has no text content and no End to clear a flag).

Upstream evidence

Not applicable — this is local OpenIOC XML parsing logic (src/tools/openioc.rs) with no MISP REST controller or PyMISP wire-protocol dependency; the bug is purely a quick-xml Event::Start/Event::Empty/Event::End state-machine defect, verifiable directly from quick-xml's documented event model (Event::Empty has no paired Event::End) rather than from the MISP/PyMISP pinned checkouts.

Verified against MISP core 843b6744 and PyMISP e7debb4f.

Tests

  • openioc_self_closed_content_does_not_leak_into_next_text

Verified locally: cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, cargo test --all-features --lib, --test integration_tests --no-run, and cargo check --examples, each in an isolated target directory.

🤖 Generated with Claude Code

The Content-handling arm matched both Event::Start and Event::Empty and
set in_content = true for either. quick-xml never emits a matching
Event::End for a self-closed element, so a self-closed <Content/> left
in_content stuck true with no corresponding reset. The next Text node
encountered anywhere in the document (e.g. unrelated whitespace/text
inside the same IndicatorItem) was then attributed to the stale
current_search, fabricating a bogus MISP attribute from non-IOC text.

Fix: only arm in_content on a real Event::Start for Content; an
Event::Empty Content contributes no text and should not open the flag.

This is local OpenIOC XML parsing logic with no MISP REST wire format
involved, so no upstream controller/PyMISP citation applies.

Added openioc_self_closed_content_does_not_leak_into_next_text, which
fails before this fix (asserts len 1, previously got 2 with a bogus
"leaked-non-ioc-text" md5 attribute) and passes after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JoSButbVaWEtGMXwTx73BZ
@elhoim
elhoim marked this pull request as draft September 3, 2026 05:04
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