feat: refang defanged IoC inputs before building requests - #327
Conversation
Threat-intel reports print indicators defanged (hxxps[:]//evil[.]com, 127[.]0[.]0[.]1). The server looks URLs up by an exact hash and stores a submitted URL verbatim, so a defanged value silently missed a search or became a broken URL artifact. Add polyswarm_api.refang (refang_text, is_network_ioc, refang_ioc) and a refang_iocs=True constructor flag. When on, the client refangs the URL/domain/IP inputs of search_url, search_by_metadata (ips/urls/domains, never the free-form query), search_by_ioc, check_known_hosts, the known-host writes, and every URL submission path. Live values are sent unchanged; refang_iocs=False sends inputs verbatim. The rules, gate and case table are shared with other PolySwarm clients; test/fixtures/refang_cases.json is kept byte-identical across them.
Python's case-insensitive flag folds Unicode (U+212A KELVIN SIGN matched "k", so "evil[.]Kom" was refanged), \s/\S cover Unicode whitespace, and "$" matches before a trailing newline - all unlike the other clients that implement the same contract. Spell letters as explicit [aA] classes, use the ASCII whitespace set, trim only that set, and fullmatch the network-IoC pattern. The shared case table grows to 39 cases pinning these.
submit already skipped the refang for preprocessing type qrcode, whose argument names an image file rather than a URL; sandbox_file did not, so a file called qr[.]png was sent as qr.png. Pin both paths on both transports, cover the async opt-out of the known-host writes and the bare-string form of the list arguments, and put the httpx_kwargs paragraph back under the constructor block in the contract spec.
…the architecture specs
|
Reviewed against 1. Spec drift — The def __init__(self, key=None, ..., *, session=None, refang_iocs=True, **httpx_kwargs): ...but the def __init__(self, key=None, ..., *, session=None, **httpx_kwargs): ...
2. Low — the live-URL gate only covers scheme-bearing inputs
Nothing else: the |
…g_iocs on the async constructor
|
Addressed in 95682a7:
|
|
Review — refang defanged IoC inputs The 1. From the Versioning table:
This wants an explicit maintainer decision rather than being folded into a minor: either default 2. The QR guard in
3. Test coverage: the uploaded-content assertion is sync-only
4. Minor: the request-shape harness bypasses the transport
|
A default-on refang changes what the write paths store (known-host rows, URL submissions), which the versioning table only allows in a major. With refang_iocs=False as the default the new keyword is purely additive; the CLI opts in through its own option. Also read the QR preprocessing type with .get() in submit, as sandbox_file already does, and cover the uploaded URL content on both transports with its opt-out twin.
|
Addressed in 1647bdd:
Full suite: 381 passed. The previous run's e2e failure was |
|
Reviewed against I hand-traced all 42 rows of Two non-blocking notes, neither a bug:
|
|
Thanks. Both notes are now in the PR body under "Two side notes", with no code change: the |
SummaryLets an analyst paste an indicator straight out of a threat-intel report. A defanged URL, domain or IP ( Severity: 0 HIGH · 1 MODERATE · 1 LOW.
Fixes are proposed, not applied; nothing was run. Cross-repo coordination
Merge order: Contracts crossing the set: none carries a finding. Coherence: fixes are independent; no cross-repo adjustment needed. Worth knowing: a third client — the web UI — implements this same contract on its own branch, with a byte-identical Findings (round 1)Every finding below is work for this change set; each entry's [MODERATE] F1. QR-code sandbox submissions upload the file path, not the imageWhat happens: An SDK caller submitting a QR-code image through When: Any Why:
Lands in: polyswarm-api Proposed fix (untested): Mirror elif artifact_type == resources.ArtifactType.URL:
if preprocessing and preprocessing.get("type") == "qrcode":
# A QR-code submission's argument is an image path, not a URL:
# read from disk, never refanged (same rule as ``submit``).
artifact = resources.LocalArtifact.from_path(
self, artifact, artifact_type=artifact_type, artifact_name=artifact_name
)
else:
artifact = self._refang(artifact)
artifact = resources.LocalArtifact.from_content(
self, artifact, artifact_name=artifact_name or artifact,
artifact_type=artifact_type,
)
Standards conformityChange-level: §15 (comment the fact, specify the design) → F2. Project-level: no non-clean rows for this repo.
Set-level: §14 clean — one externally-facing capability rather than a layer, and no UI change merging ahead of it. Branch-name identity clean: |
mjbradford89
left a comment
There was a problem hiding this comment.
Reviewed. Open: 1 MODERATE (F1 — sandbox_file's QR-code branch uploads the image path as the artifact body instead of the file; pre-existing behaviour that this PR newly documents as working) and 1 LOW (F2). Details in the review comment above.
SummaryLets an analyst paste an indicator straight from a threat-intel report ( Severity: 0 HIGH · 2 MODERATE · 4 LOW. Prior feedback: 23 checked · 2 open.
Fixes are proposed, not applied; nothing was run. Cross-repo coordinationSet: polyswarm-api#327 merged · SDK (reference) ← you are here — polyswarm-cli#274 merged · CLI — the web UI's PR (private repo) open · UI Merge order:
Coherence: the fixes are independent across repos. The web UI's three LOW fixes share two passages there and are applied together. Findings (round 2)Every finding below is work for this change set; each entry's [MODERATE] F2. QR-code
|
| Status | Raised | The ask | Disposition |
|---|---|---|---|
| not addressed | #327, round 1 | QR sandbox_file uploads the path, not the image |
→ F2 |
| not addressed | #327, round 1 | trim refang.py's docstring to point at the spec |
→ F3 |
Standards conformity
The round 1 audit stands. There is no new change-level violation: F3 is the §15 item it already recorded.
Set — Clean: §14 (one capability, three clients, SDK and CLI merged first). Rule 6 name identity isn't load-bearing here, because no CI seam resolves the web UI against the SDK or CLI.
|
F2, F3 and this repo's half of F4 are addressed in #328 (re-created |
What
The client can now refang defanged indicators of compromise before it builds a request, so
hxxps[:]//evil[.]comis sent ashttps://evil.comand127[.]0[.]0[.]1as127.0.0.1. The helpers are public in the new modulepolyswarm_api.refang:refang_text,is_network_iocandrefang_ioc(value, accept=None). A new constructor keyword,refang_iocs, switches it on for both clients. It is opt-in (defaultFalse): without it every input is sent verbatim, exactly as in 4.5.0.Why
Threat-intel reports print indicators defanged so that nobody clicks one by accident. Analysts paste them straight from the report, and a defanged value never works: the server looks URLs up by an exact hash of the string, so a search silently finds nothing, and it stores a submitted URL verbatim, so a submission creates a new, broken URL artifact. The server deliberately does not guess what a string was meant to be, so each client refangs at its own edge, before the request exists.
Exactly which inputs are touched
With
refang_iocs=True:search_url(url)search_by_metadata(ips=, urls=, domains=). The free-formqueryis never touched.search_by_ioc(ip=, domain=)check_known_hosts(ips=, domains=)hostofadd_known_good_host,add_known_bad_hostandupdate_known_good_host. A defanged host written to the known-host catalogue would never match a real lookup, so it gets the same treatment as a search.submit(..., artifact_type=URL),sandbox_file(..., artifact_type=URL)andsandbox_url(url). Both the uploaded content and the default artifact name are refanged; an explicitartifact_nameis kept as given.Never touched: hashes, ids, and the argument of a QR-code submission (
preprocessing={'type': 'qrcode'}onsubmitorsandbox_file). That argument names an image file, not a URL, soqr[.]pngstaysqr[.]png.The rewrite is gated so that it can only ever fix an indicator, never damage one. It applies only when:
[.]inexample.com/a[.]borhttps://example.com/a[.]bsurvives), andOtherwise the value is sent byte-for-byte as before.
Opt in with
PolyswarmAPI(..., refang_iocs=True). The setting is also readable as the public attributeapi.refang_iocs, so a consumer that handles a value outside the endpoint methods can applyrefang.refang_iocunder the same switch.Two side notes
submitnow reads the QR preprocessing type withpreprocessing.get("type"), assandbox_filealready did. This is a small behaviour change: apreprocessingdict with notypekey used to raiseKeyErrorand now takes the regular URL-content branch.ips=/urls=/domains=is refanged, but onlycheck_known_hostsaccepts that form.search_by_metadatahas always iterated such a string one character per param, and this PR does not change that.A contract shared with other clients
Other PolySwarm clients implement the same refang contract: the same rules in the same order, the same gate, and the same case table.
test/fixtures/refang_cases.jsonis kept byte-identical across them, so changing a rule means changing it everywhere. Portability is part of the contract:k);\b,\d,\w,\sor\S; whitespace is the explicit ASCII set;fullmatch, because$also matches before a trailing newline.Out of scope: email
[at], a bare-worddot,http__host/http:\\host, stripping bare brackets (IPv6 literal syntax), and non-ASCII hosts.Version: 4.6.0, and why refanging is opt-in
Refanging on by default would not be additive. It changes what the write paths store: a known-host row written verbatim as
evil[.]comby an earlier client would stop matching a lookup that now sendsevil.com, and a URL submission would create a different artifact than the same call did on 4.5.0. The versioning table inspecs/05-downstream-contract.mdonly allows a new keyword in a minor release when its default preserves current behaviour, so the default isFalseand 4.6.0 is a minor bump. The CLI opts in through its own--refang/--no-refangoption. The bump lands in this feature PR under the standing exception inAGENTS.md: the CLI resolves this SDK from source by branch name, and its paired change raises its floor topolyswarm_api>=4.6.0, a version this repo must already declare. The emitted string is a clean4.6.0, with no dev suffix.Tests
polyswarm_api.refang, driven by the shared 42-case table._paginate/_singleboundary and assert the outgoing params or JSON body. They also cover the uploaded URL content on both transports (on and off), the QR-code paths, and the bare-string form of the list arguments.