Repository navigation
feat: [glimps_malware] add analysis lookup and sample submission - #1067
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 757039156a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| entries = [entry for entry in archive.infolist() if not entry.is_dir()] | ||
| if len(entries) != 1 or entries[0].file_size > limit: | ||
| raise ModuleError("A malware-sample archive must contain one file within max_size_mb.") |
There was a problem hiding this comment.
Ignore the MISP filename sidecar
Current MISP malware-sample ZIPs contain both the sample and a <md5>.filename.txt metadata member, as documented in misp_modules/modules/expansion/bingraph.py and exercised by existing upload-module tests. This comprehension counts both as files, so the subsequent len(entries) != 1 check rejects normal two-member malware-sample archives before lookup or submission. Exclude the filename sidecar when selecting the sample member.
Useful? React with 👍 / 👎.
| if not isinstance(encoded, str) or len(encoded) > 2 * limit: | ||
| raise ModuleError("Missing or oversized base64 sample data.") | ||
| content = base64.b64decode(encoded, validate=True) | ||
| filename = attribute["value"].split("|", 1)[0].replace("\\", "/").rsplit("/", 1)[-1] |
There was a problem hiding this comment.
Validate the unpacked sample against its declared MD5
For a malware-sample, the value is filename|md5, but this line discards the MD5 and the unpacked content is never checked against it. If an archive is stale, corrupted, or paired with the wrong attribute value, the module looks up or submits one binary while linking the result to an attribute identifying another binary, corrupting the enrichment's provenance. Parse and validate the declared MD5 after unpacking, as the other malware-sample consumers do.
Useful? React with 👍 / 👎.
| REQUIRED = ["api_url", "api_token"] | ||
|
|
||
|
|
||
| def _expand(event, attribute, value, config, limit): |
There was a problem hiding this comment.
Honor max_results during result import
When an analyst configures max_results, the handler validates and passes it here, but _expand never reads limit; the complete provider payload is always serialized into the sandbox report. Consequently even max_results=1 has no effect on responses containing large result collections, contrary to the exposed and documented configuration. Apply the limit to imported result collections or remove the ineffective option.
Useful? React with 👍 / 👎.
Analysts can retrieve GLIMPS Detect results by SHA-256 or analysis UUID. Supplied samples are hashed and checked before an explicitly enabled submission. Sandbox reports retain provider verdicts and analysis details while excluding the expert-view access token. Receipts provide an analysis UUID for a later lookup.
Includes a usage guide and focused adapter tests.
Validation:
19 passed, 1 warning in 0.68swithpytest -q tests/test_glimps_malware.py; flake8 passed for the module. The combined enrichment regression check passed 389 tests.Authenticated provider API validation remains pending. Tests use sanitized fixtures; no provider credentials are included.
Review updates: Handle current MISP sample ZIP sidecars and verify declared MD5. Remove the ineffective result-count setting and classify reports using the GLIMPS provider relation.
Updated validation: 22 passed, 1 warning in 0.61s; module lint passed. Combined regression validation passed 409 tests.
Companion provider suggestions: MISP/misp-objects#552. The adapters use the existing sandbox-report relations.