Detect new malware families and release 1.8.1 - #49
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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe change adds post-analysis malware detection rules, two threat-family definitions, rule registration, classifier and integration tests, quarantine coverage, and coordinated version updates from 1.8.0 to 1.8.1. ChangesThreat detection and classification
Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant AssemblyScanner
participant RuleFactory
participant ScanRules
participant ThreatFamilyClassifier
participant ScanResultMapper
AssemblyScanner->>RuleFactory: CreateCoreRules()
RuleFactory->>ScanRules: Execute initial and post-analysis rules
ScanRules-->>AssemblyScanner: Return findings
AssemblyScanner->>ThreatFamilyClassifier: Classify findings and hash
ThreatFamilyClassifier-->>AssemblyScanner: Return family and variant
AssemblyScanner->>ScanResultMapper: Map scan result with raw bytes
ScanResultMapper-->>AssemblyScanner: Return disposition and match details
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit reads each line, Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77a201417c
ℹ️ 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".
| foreach (var namespaceGroup in module.Types | ||
| .Where(static type => !string.IsNullOrWhiteSpace(type.Namespace)) | ||
| .GroupBy(static type => type.Namespace, StringComparer.Ordinal)) |
There was a problem hiding this comment.
Removing or omitting namespaces is enough to bypass both new behavior-family detections: when all relevant types have an empty namespace, this filter creates no groups, so CoordinatedPayloadDeliveryRule never emits the coordinator finding required by either family matcher. Hashless builds of the same droppers would therefore evade the behavior-based classification this change is intended to provide; group global types under an explicit assembly-level scope instead of discarding them.
Useful? React with 👍 / 👎.
| var namespaceTypes = namespaceGroup.SelectMany(EnumerateTypeAndNested).ToList(); | ||
| var methods = namespaceTypes | ||
| .SelectMany(EnumerateMethods) | ||
| .Where(static method => method.HasBody) | ||
| .ToList(); |
There was a problem hiding this comment.
When unrelated types share a namespace, all of their methods, calls, and literals are pooled before the later Any checks. A normal application can therefore combine an updater that downloads/extracts a ZIP, a separate component that writes a Run key, and another helper that launches with hidden settings into a Critical coordinator finding; the Pawns matcher then labels it a KnownThreat and recommends blocking. Require these signals to belong to a connected call graph or otherwise proven execution cluster rather than merely the same namespace.
Useful? React with 👍 / 👎.
| bool hasTransform = method.Body.Instructions.Any(static instruction => instruction.OpCode == OpCodes.Xor) || | ||
| calls.Any(called => | ||
| called.Name.Contains("Xor", StringComparison.OrdinalIgnoreCase) || | ||
| called.Name.Contains("Encrypt", StringComparison.OrdinalIgnoreCase) || | ||
| called.Name.Contains("Encode", StringComparison.OrdinalIgnoreCase)); |
There was a problem hiding this comment.
When a method reads Environment.MachineName, performs any unrelated XOR/encrypt/encode operation, and makes any network send, these independent booleans produce a High DataExfiltrationRule finding even though no value is shown to flow from the host-data source through the transform to the send. For example, an ordinary telemetry or registration method that URL-encodes a machine name and calls HttpClient.SendAsync is reported as exfiltration regardless of destination; verify the operand flow into the request rather than treating mere co-occurrence as a transfer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a031f7c66
ℹ️ 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".
| (declaringType == "System.IO.Compression.ZipFile" && | ||
| methodName == "ExtractToDirectory") || |
There was a problem hiding this comment.
When an installer or updater calls ZipFile.ExtractToDirectory and later writes ordinary installation state with RegistryKey.SetValue in the same method, this new Transform classification satisfies HasTransform plus HasRegistrySink in DataFlowPatternEvaluator, producing an ObfuscatedPersistence finding that seeds a Suspicious, blocking disposition. ExtractToDirectory is a void filesystem operation and does not transform the value written to the registry, so model archive extraction separately or require an actual value-flow relationship before treating it as an obfuscation transform.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
newauthquarantine sample and preserves Rexon findings without premature family assignment.ZipFile.ExtractToDirectoryoperation classification.1.8.1.MLVScan.Core.1.8.1.nupkgcreation succeeded.