Skip to content

fix: [openioc] require both search-contexts before merging a composite#1449

Open
arpitjain099 wants to merge 1 commit into
MISP:mainfrom
arpitjain099:chore/openioc-composite-indicator
Open

fix: [openioc] require both search-contexts before merging a composite#1449
arpitjain099 wants to merge 1 commit into
MISP:mainfrom
arpitjain099:chore/openioc-composite-indicator

Conversation

@arpitjain099

Copy link
Copy Markdown

While going through the OpenIOC importer I hit a case where an AND-indicator with two children that aren't a real composite pair gets turned into a bogus merged attribute. The composite check reads if (value1 and value2) in childList, but Python evaluates value1 and value2 first, which just returns value2, so only value2 is actually checked against the indicator's search-contexts. If a mapping key's second half happens to match, the two unrelated children are merged into a single type='other' attribute (and emitted once per matching mapping key, so it also duplicates), instead of coming through as their two correct individual attributes.

The fix is to check that both search-contexts are present before merging: if value1 in childList and value2 in childList. Genuine composites still merge exactly as before.

I added tests/test_openioc.py covering both paths: a non-composite two-child AND-indicator (RegistryItem/Value + FileItem/Md5sum) now yields the two individual regkey and md5 attributes, and a real composite (FileItem/FileName + FileItem/Md5sum) still merges into the single filename|md5 attribute. The new test fails on the current code and passes with the change; the rest of the offline suite is unaffected.

Thanks for taking a look.

The composite check in set_all_attributes used
`if (value1 and value2) in childList`, which Python evaluates as
`(value1 and value2)` first. Both operands are non-empty mapping-key
strings, so the expression reduces to `value2 in childList` and value1
is never checked. An AND-indicator with two children whose combined pair
is not a real composite (but whose second search-context matches a
composite mapping's value2) was wrongly merged into a bogus type='other'
attribute, emitted once per matching mapping key.

Check that both search-contexts are present before merging, so genuine
composites still merge and unrelated pairs fall through to their two
individual attributes. Adds a regression test covering both cases.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@Rafiot Rafiot self-assigned this Jul 1, 2026
@adulau

adulau commented Jul 3, 2026

Copy link
Copy Markdown
Member

We are currently doing a survey who is still using OpenIOC format (as we want to remove it from MISP). Do you still really use OpenIOC format?

@arpitjain099

Copy link
Copy Markdown
Author

Thanks for the heads up on the survey. Honestly, no, we don't have a production dependency on OpenIOC. This came out of reading through the importer rather than a real workload, I just noticed the composite-merge check was collapsing two unrelated children and sent a fix. So if the survey points toward dropping OpenIOC, please go ahead and close this, no objection at all.

If you do want to keep the importer around for now, the fix still stands on its own. The red CI is on me: the new tests/test_openioc.py has a stray # type: ignore on the bs4 import that mypy flags as unused in your setup. The tests themselves pass (115 passed). It's a one-line cleanup and I'm happy to push it, just let me know which way the survey lands.

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.

3 participants