Repository navigation
Fix ScanMalware attribution to subdomain tenants - #953
Merged
Ian Hellen (ianhelle) merged 3 commits intoOct 15, 2026
Merged
Ian Hellen (ianhelle) merged 3 commits into
Ian Hellen (ianhelle) merged 3 commits into
Conversation
The scan list for a host also holds its subdomains. On a shared platform a subdomain is an unrelated tenant, so a lookup of netlify.app or workers.dev took a tenant phishing page's verdict (Severity high), and -quick attached the tenant's scan as LastScanUrl. A scan is now selected only when its own host is the observable's host or its www name, in addition to matched_on containing "url". URL lookups already required the exact URL and are unchanged.
Ian Hellen (ianhelle)
requested changes
Oct 15, 2026
Ian Hellen (ianhelle)
left a comment
Ian Hellen (ianhelle)
left a comment
Contributor
There was a problem hiding this comment.
The stricter host attribution addresses the shared-platform issue, but the new host guard introduces a trailing-dot normalization regression that can suppress a matching threat verdict. Please normalize both hostnames consistently and add the regression coverage described inline before merging.
The host guard stripped a trailing dot from the observable's host but not from the scanned URL's host. A scan of https://example.com./, looked up as that same URL, compared example.com with example.com. and was skipped, so its verdict was never requested and the result stayed informational. Both hosts are now normalized the same way. The exact-URL check for URL observables is unchanged. Two regression tests cover the dotted URL as both observable and scan, and a domain observable whose scan URL has the dot. Found in review by Ian Hellen.
Ian Hellen (ianhelle)
approved these changes
Oct 15, 2026
Contributor
|
Nice! Thanks Jonas 💯 |
stanfrbd
pushed a commit
to stanfrbd/cyberbro
that referenced
this pull request
Oct 18, 2026
The host check stripped a trailing dot from the observable's host but not from the scanned URL's host. A scan of https://example.com./, looked up as that URL (or as example.com), compared example.com with example.com. and was skipped, so its verdict was never reported. Both hosts are now normalized the same way; the exact-URL check is unchanged. The same asymmetry was found in review of the sibling msticpy fix (microsoft/msticpy#953).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
For a
dnsorhostnamelookup, the ScanMalware provider picks the newest completed scan whosematched_oncontainsurl. The scan list for a host also holds its subdomains, and on a shared platform a subdomain is an unrelated tenant. So in v3.1.0 a lookup ofnetlify.apporworkers.devreturns Severity high, Verdict Malicious, taken from a tenant's phishing page, and-quickattaches that tenant's scan asLastScanUrl.A scan is now selected only when its own host is the observable's host, or its
www.name, as well asmatched_oncontainingurl. This builds on cd856f7, which movedLastScan/LastScanUrl/LastTitleonto the selected scan; this change narrows which scan is selected. URL lookups already required the exact URL and are unchanged, as are IP lookups.Disclosure: I run ScanMalware and wrote this provider (#938).
Before and after, live against scanmalware.com
Through
ScanMalware().lookup_ioc(...), v3.1.0 against this branch:netlify.appLastScanUrla tenantworkers.devLastScanUrla tenantgithub.iohttps://excelformulabot.github.ionetlify.app(-quick)LastScanUrla tenantexample.comhttps://gilded-macaron-79e0e2.netlify.app/(url)ResultstaysTrueandScanskeeps the full count; only the scan the details and verdict come from is stricter.Tests
tests/context/test_scanmalware.pygains 8 test cases (22 to 30). Five fail onmain: a subdomain tenant (with and without-quick), the platform's own scan behind a newer tenant scan, a lookalike host (example.com.ruunderexample.com), and a deeper subdomain under ahostname. Three guard against over-tightening: thewww.name, letter case and an explicit port still count.pytest tests/context/test_tiproviders.py tests/context/test_scanmalware.py: 71 passedruff check msticpy --ignore PLW0603: cleanpylintwith the CI flags: 10.00/10mypywith the CI flags: no issuesThe same rule is in the MISP expansion module (MISP/misp-modules#916, released in v3.0.11).