Skip to content

[high] fix: [domaintools] propagate service-call errors from handler - #867

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/055-domaintools-swallowed-error
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/055-domaintools-swallowed-error

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — domaintools discards service-call return values, so failed API lookups look like empty results.

  • Problem — The handler() loop in misp_modules/modules/expansion/domaintools.py calls each requested service function but discards the return value, so when a service signals failure by returning the misperrors dict the error is thrown away and the loop continues.
  • Fix — Captures each return value and returns misperrors immediately when a call has failed.
  • Effect — A failed DomainTools lookup (bad credentials, quota exceeded, upstream error) is reported as an error instead of partial or empty results that an analyst reads as no data for the indicator.

domaintools.py's handler() calls each requested service function in a loop but discards the return value:

for s in services:
    globals()[s](domtools, to_query, values)

Every service function (parsed_whois, domain_profile, reverse_whois, host_domains, reverse_ip_whois) signals an API failure by returning the module-level misperrors dict; on success it returns the values accumulator instead. Because the loop never looks at the return value, a failed call's error is thrown away and the loop simply moves on.

Impact on an analyst: if a DomainTools API call fails (bad credentials, quota exceeded, upstream error), the module does not report the failure. Instead it returns {"results": values.dump()} with whatever partial data was collected before the failure — which can be empty. This looks to the analyst like "DomainTools has no data for this indicator" rather than "the lookup failed," so a real API problem is silently mistaken for a negative enrichment result.

Fix: capture each service call's return value and, if it is misperrors (an identity check, since the success path returns the separate values object), return misperrors immediately instead of continuing the loop.

This does not change any documented contract — returning misperrors on failure is exactly what the finding calls for and what the other error paths in this module already do; it simply stops that signal from being dropped.

Verification

  • flake8 on the changed file: clean (exit 0)
  • Full module test suite: 161 passed, 4 skipped, 5 subtests passed in 43.44s

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

🤖 Generated with Claude Code

handler() iterates the DomainTools service functions (parsed_whois,
domain_profile, reverse_whois, host_domains, reverse_ip_whois) and
discards each call's return value. Every one of these functions
signals a failed API call by returning the module-level misperrors
dict, but since handler() never inspects that return, a failed
lookup falls straight through to `return {"results": values.dump()}`
with whatever partial values were collected before the failure -
an analyst sees an empty or partial success instead of the actual
DomainTools error (bad credentials, rate limit, API outage).

Capture each service call's return value and, when it is the
misperrors object, return it immediately instead of continuing the
loop.

Verified with flake8 (clean) and the full pytest suite against a
freshly started modules server on port 6755: 161 passed, 4 skipped,
5 subtests passed, matching the documented baseline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8
@elhoim elhoim changed the title fix: [domaintools] propagate service-call errors from handler [high] fix: [domaintools] propagate service-call errors from handler 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