Repository navigation
fix(yara): require corroborating webshell markers - #489
Conversation
`php_webshell_known` matched the bare substrings "behinder" and "WSO " under `any of them`, so ordinary prose produced a CRITICAL YR2 finding at 0.9 confidence with a remediation telling the reader to remove a webshell. The German words "behindert" and "Behinderung" contain the first string, and the product name "WSO 2 Micro Integrator" contains the second, as reported in #487. Behinder is now identified by its hardcoded AES key, md5("rebeyond") truncated to 16 characters, which its PHP, ASP and JSP shells share and which signature-base [1] uses for the same purpose. The key is written as a YARA hex string so the packaged rule file does not carry the indicator in plaintext, in the spirit of the encoded malware rules. WSO is identified by the helper names oRb introduced in 2.x: `wsoEx(`, `WSO_VERSION` and `wsoSecParam`. I checked a WSO 2.5 source against the old and new rule: the old one hit only the banner and the bare "WSO " string, the new one hits the banner and all three helpers, so no known sample is lost. I preferred narrowing the strings over a `fullword` modifier because a document that names the Behinder family would still have scored CRITICAL, and "WSO 2" would still have matched. Fixes #487 [1]: https://github.com/Neo23x0/signature-base Signed-off-by: Stefano Amorelli <[email protected]>
The benign cases are the two reproductions from #487 plus a document that names the Behinder and WSO families without shipping them. The malicious cases are a Behinder PHP shell, a Behinder JSP shell and a WSO fragment carrying `WSO_VERSION` and `wsoEx(`. All six fail against the previous rule: the prose cases fired and the samples were missed, so the tests guard both directions. The samples are base64-encoded like the existing reverse shell fixture so the indicators do not sit in the repository in plaintext. Signed-off-by: Stefano Amorelli <[email protected]>
Group family-specific indicators so isolated constants and documented keys do not produce critical findings. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <[email protected]>
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Approved at head bdf41a8d97f2d21b941269cc782fb5bffa33035e.
The revised rule keeps the established high-specificity family markers while requiring corroboration for the newly introduced WSO and Behinder indicators. The tests cover benign prose, each isolated marker, mixed case, and representative PHP/JSP family samples, so they would detect the false-positive regression addressed here.
This supersedes the incomplete signature change in #488; do not merge both. Merge gate: GitHub reports no checks and merge state BLOCKED, so required CI/branch-protection status must become known and successful before merge.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 6b5ee6496951e21c4d5726057a85b530df78147e after the intervening main synchronizations. The two PR-owned blobs are unchanged from the previously assessed head. The rule still removes the prose-colliding bare family names, requires corroborating WSO markers or the Behinder key plus a PHP/JSP server-language tag, and the regressions cover benign prose, isolated indicators, mixed case, and representative family samples. I found no required code or test changes.
No hosted checks are attached to this exact head and GitHub reports merge state BLOCKED; do not merge until required CI is present and successful and branch protection is satisfied. This PR supersedes #488, so the two alternatives must not both be merged.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 3b1a95673654d3999df665e5fb03946363a0315a. The YARA feature blob remains identical to reviewed SHA 6b5ee6496951e21c4d5726057a85b530df78147e; test-file drift is only an unrelated upstream timeout-test update. The corroborated WSO/Behinder signatures remove prose collisions while retaining representative family detections. I found no required change.
No required checks are attached and GitHub reports mergeStateStatus=BLOCKED; do not merge until current-head CI and branch protection pass, and do not merge both this PR and superseded #488.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed current head 6d309d8b9f42de160a789a9bf750f69166452bb8 after the automatic #558 synchronization. The intervening delta is only the reviewed batch-scan change from main; both PR-owned blobs remain unchanged from the previously assessed implementation. The YARA rules still replace prose-colliding family names with corroborated WSO and Behinder markers, and the tests retain benign prose, isolated indicator, mixed-case, and representative PHP/JSP coverage. I found no required change.
No hosted checks are attached to this exact head and GitHub reports BLOCKED, so branch protection and required CI prevent merging. This PR supersedes #488; do not merge both alternatives.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed the current head after main synchronization, including the complete two-file feature diff, complete rule context, inert decoded fixture text, prior review history, and inherited main delta. The PR-owned files are unchanged by the latest merges. WSO requires two corroborating identifiers; Behinder requires its family key plus PHP/server-side syntax, while the other established family markers remain unchanged. Negative controls cover prose and isolated indicators; PHP/JSP and case variants cover retained positives. No required change found.
The new main-only changes are the already-reviewed #539/#588/#591/#607/#597. No exact-head CI is attached and the merge state is not ready. This remains an alternative to #488, not a reason to merge both.
Static review only; no contributor code/tests executed.
Reviewed head: 36314736cd62d1c95c0d6c378a913d1ae14b5f78.
Priority: P1 — Reduces critical malware false positives without single-marker exemptions.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed exact current head 9cda51634be90c59c1923e8a273a8746ef327715. The previous review text named 36314736cd62d1c95c0d6c378a913d1ae14b5f78, so I did not rely on its API commit association alone. Both PR-owned rule/test blobs are byte-identical between those commits; I also read the complete current two-file diff, surrounding rules/test harness, fixtures, and prior discussion.
The WSO signature requires two corroborating identifiers, and Behinder requires its key plus server-language syntax, while established family signatures remain intact. Regression assertions cover benign prose, isolated markers, mixed case, and representative PHP/JSP positives. No required change found.
No hosted checks are attached to this exact head in the collected status, so current-head CI/branch rules still need to pass before merge. This remains an alternative to #488; do not merge both blindly. Static inspection only; no contributor code/tests executed. This approval applies explicitly to the current head above. No merge performed.
3 participants
Summary
This builds on #488 and preserves the original author attribution while tightening the single-marker cases found during validation.
Fixes #487
Validation
make lintmake format-checkmake docker-smokeSubmitted by Codex on behalf of Mohit Gupta.