Skip to content

fix(yara): avoid ransomware findings for Bitcoin payment prose - #608

Merged
rng1995 merged 8 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-606-ransomware-context
Oct 8, 2026
Merged

rng1995 merged 8 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-606-ransomware-context

Conversation

@chrisknvidia

@chrisknvidia chrisknvidia commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Bitcoin wallet and payment documentation could receive a Critical YR1 ransomware finding from payment-only text. Require specific ransomware context and filter only direct negations of built-in payment demands, while preserving actionable continuations, custom rules, and other encryption/rename/overwrite indicators.

Filter individual raw occurrences before choosing evidence, locations, and fingerprints, so a preceding benign notice cannot own a later actionable finding. The follow-up integrates current main and compares rendered Markdown with MarkdownIt in both report assertion paths, preserving escaped report source.

Validation on 136c99da:

  • 388 native YARA and graph cases plus 442 report/rendering/occurrence cases passed; lint and formatting passed.
  • 15 real-file CLI invocations passed across JSON/Markdown, benign/positive/mixed inputs, and baseline generation/suppression rescans. Finding fingerprint and source location remained stable. Two fresh subprocess CLI scans also passed; another cold startup exceeded its local deadline during dependency imports, so the larger CLI matrix used one isolated process without graph or provider mocks.
  • Independent code and security-focused reviews found no actionable issue.

Hosted unit checks on the updated head are running. No hosted LLM or production inference is claimed for this static detector change.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Hi @chrisknvidia, thank you for your contribution to SkillSpector — we really appreciate the time you put into this! A few items need attention before it can be merged; details below.

Reviewed exact head bc59cbf68c946229ce2e795cb34c50340d135638. For #606, the PR replaces $ransom_note's broad pay\s+.*bitcoin|send\s+.*btc branches with four same-line strings (decoded malware.yar lines 79–82). Each requires a payment phrase together with specific extortion wording. The encrypted-files notice and the walk/rename/overwrite indicators are unchanged. I decoded and diffed the base and head resource: the CRLF endings are preserved, nothing else moved, and there are no new YARA compiler warnings. I read both test files and scanned my own samples with the decoded rules in yara-python 4.5.4; contributor code and tests were not run. The #606 wallet text and the other benign sentences are cleared, and my harness agrees with every test expectation. However, the narrowing removes detection of common ransom notes, and the tests lock one of them in as benign.

Findings

  1. [Blocking] Common ransom notes are no longer detected. Outside the unchanged your files … encrypted notice, the payment branches now fire only when one of these appears on the same line within 80–160 characters of the payment:

    • to (receive|get|obtain) (the|your) decryption key
    • we will (publish|leak|delete|destroy) your …
    • your (files|data) will be (deleted|destroyed|published|leaked)
    • you must / need to / have to / please pay (the|a) ransom

    Real notes usually split the demand across lines or use other verbs. Each of the following is ransomware_behavior on base; at head no built-in YARA rule fires:

    • Send 0.5 BTC to <addr> to decrypt your files.
    • Pay 1 bitcoin within 72 hours or your files will be lost forever.
    • To recover your files, pay 0.3 Bitcoin to <addr>.
    • Send 0.5 BTC to <addr>. followed by To receive the decryption key, email us your personal ID. on the next line.
    • Your documents, photos and databases have been encrypted. followed by To decrypt them, send 0.5 BTC to <addr>.
    • A five-line note with Send 0.5 BTC to <addr>, After payment you will get the decryption key. and … your files will be deleted. on separate lines.

    A complete multi-line Python ransomware script is also missed. It walks ~, overwrites each file with Fernet.encrypt, and writes the note Your documents have been encrypted. / Send 0.5 BTC to <addr> to decrypt your files.. $walk_encrypt and $mass_overwrite only match when os.walk is on the same line as the encrypt/open call, so on base the note was this rule's only indicator.

    Please restore ransom-note recall. Some options:

    • Allow a bounded multi-line window between the payment and the ransom context (e.g. /s with .{0,300}).
    • Broaden that context to decrypt/restore/recover/unlock your files/data/documents, decryptor, locked, and lost.
    • As #606 suggests, gate the generic payment phrase in the condition on another ransomware indicator in the file.
  2. [Blocking] The tests lock in a false negative. Two tests assert that "Your data has been encrypted.\nPay the ransom in Bitcoin." is not ransomware, even though it is a plain two-line ransom note:

    • tests/nodes/analyzers/test_static_yara.py:628
    • tests/integration/test_ransomware_rule.py:47 (id="multiline-bare-imperative")

    test_static_yara.py:634 ("Send BTC\nTo receive the decryption key, contact support.") does the same for the classic layout of a payment line followed by a key line. Please turn these into positives. Add positive regressions for the notes in finding 1, including an encrypt-and-drop-note script. Keep the #606 wallet text, the security-advice sentences and the distance-limit cases as negatives.

  3. [Non-blocking, pre-existing] $ransom_note still flags cloud-storage prose. Your files are encrypted at rest and in transit with AES-256. remains CRITICAL on both base and head. Consider excluding at rest/in transit, either in this ransomware false-positive fix or in a follow-up.

Merge/interaction: The PR merges cleanly with main. #489 touched only webshells.yar and the test file (auto-merges, no interaction with ransomware_behavior). #593, #594 and #599 also re-encode malware.yar.b64; merging #608 with #593 conflicts inside the base64 file, so whichever lands second must decode, re-apply and re-encode.

Tests/CI: All six hosted checks pass. The new negatives fail on base, so they are meaningful. However, the positive matrix only uses the narrow vocabulary the rule now requires, so it cannot catch the recall loss above.


Decision: Changes Requested (reviewed head bc59cbf68c946229ce2e795cb34c50340d135638)

Comment thread tests/nodes/analyzers/test_static_yara.py Outdated
Comment thread tests/integration/test_ransomware_rule.py
Comment thread tests/nodes/analyzers/test_static_yara.py

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Hi @chrisknvidia, thank you for the quick turnaround and for turning every note from the last review into a positive regression!

Value and readiness: The head now catches every ransom note listed last round, including the cross-line notes and the encrypt-and-drop script. The two pinned false negatives are now positive tests. The #606 wallet text and the "encrypted at rest" prose are still cleared. However, two changes in 5edea3b miss real ransom notes that main detects:

  • the new end-of-sentence requirement on $ransom_note;
  • the negation filter, which ordinary ransom wording can trigger.

Both fixes are small, but the PR is not ready until they are made.

Previous findings (review at bc59cbf)

  • [Blocking] Common ransom notes no longer detected: Resolved for every listed case:

    • the three single-line notes;
    • payment line followed by a decryption-key line;
    • documents-encrypted line followed by a payment line;
    • the five-line note;
    • the os.walk/Fernet script.

    All of them match at the YARA layer, and the negation filter does not remove them.

  • [Blocking] Tests lock in false negatives: Resolved. test_static_yara.py now treats both notes as positives. multiline-bare-imperative expects a finding, and multiline-payment-key was added.

  • [Non-blocking] "encrypted at rest" prose: Resolved for that sentence, but the fix causes finding 1.

Material findings

  1. [Blocker] Decoded malware.yar line 78 (src/skillspector/yara_rules/malware.yar.b64): $ransom_note now requires encrypted to be followed by ., !, ?, a newline or end of data. Main matched the phrase wherever it appeared. Each note below is detected on main and at bc59cbf but gets no finding at head:

    • All of your files are encrypted with RSA-2048 and AES-128 ciphers. (Locky)
    • All your files have been encrypted due to a security problem with your PC. (Dharma/Phobos). The full Dharma note, which continues "You have to pay for decryption in Bitcoins", is also missed. \b(btc|bitcoin)\b does not match the plural Bitcoins.
    • Your files have been encrypted by LockBit 3.0
    • Your files have been encrypted, and the only way to get them back is to pay us.
    • Notes that ask for XMR rather than Bitcoin. There, $ransom_note was the only indicator.

    The terminator does not fix the broader prose false positive either: Make sure your files are encrypted. is still CRITICAL. Please restore main's match regardless of what follows, at least for have been encrypted, and exclude only the at rest/in transit continuation. Alternatively, leave that pre-existing false positive for a follow-up. Add positives for the with, by, due to and comma continuations, and consider accepting bitcoins.

  2. [Blocker] src/skillspector/nodes/analyzers/static_yara.py:96-100: _DIRECT_RANSOMWARE_NEGATION allows up to three words between the negator and pay/send, so the negation can belong to a different verb. Both of these match $payment_then_context, and the filter then drops both instances, leaving no finding (main reports both):

    • Don't forget to send 0.5 BTC to <addr> to decrypt your files.
    • Do not hesitate to pay 1 bitcoin to restore your files.

    A ransom author can skip every payment branch by starting the demand with "Don't forget to". Please treat the payment as negated only when the negator applies to it directly, e.g. never|do not|don't|must not|should not, optionally followed by ever. Add these two notes as positives next to the existing security-advice negatives.

  3. [Non-blocking] The PR description is out of date. It still says "bounded, same-line ransom context" and cites CI at bc59cbf. The head actually:

    • uses a cross-line [\s\S]{0,160} window;
    • adds a Python post-filter for negated payment wording;
    • changes how $ransom_note ends.

    Please update it so the squash commit message is accurate.

  4. [Non-blocking, pre-existing] Some benign text is still CRITICAL on both main and head. Optional follow-up:

    • backup or decryption docs followed within 160 characters by a donation line (To restore your files, run ... then Send BTC to <addr>);
    • Pay the invoice in bitcoin. Your data will be deleted after 30 days...;
    • Make sure your files are encrypted.

PIC tradeoffs: None identified.

Verification and gaps:

  • Merge commit: 5edea3b merges main and also carries the review fixes. Diffed against the merge base 8831219, the PR still touches only its four files, so the merge did not pull in unrelated changes.
  • YARA rules: I decoded malware.yar at main, bc59cbf and head. Line endings stay CRLF, and only the ransomware_behavior strings change. I compiled them with yara-python 4.5.4 (no new compiler warnings) and matched 19 ransom-note and 9 benign samples of my own, scanned as text only.
  • Negation filter: for finding 2, I re-evaluated the PR's negation regex in isolation on the YARA match instances; the PR module was not imported. Contributor tests were not run, per policy.
  • Scan cost: 0.44 s per MB on realistic documentation (main: 0.01 s), and at most 1.6 s per MB on crafted input. Acceptable.
  • CI: all 6 checks green.
  • Overlap: #593, #594 and #599 also re-encode malware.yar.b64. Whichever lands second must decode, re-apply and re-encode instead of merging the base64 text.

Decision: Changes Requested (reviewed head 5edea3b47322b1ef4a884ed0273dbcdc71239c68)

Comment thread src/skillspector/nodes/analyzers/static_yara.py Outdated
Comment thread tests/nodes/analyzers/test_static_yara.py
chrisknvidia and others added 3 commits October 5, 2026 13:39
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com>
…tion

Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the complete production and decoded YARA rule diff, all prior discussions, and the real-YARA/offline graph reporting paths. The previous detector feedback is addressed; the two remaining old inline threads are fixed and can be resolved. The exact-head relevant suite passes all 388 tests, and an independent 153-case native YARA suite and boundary probes pass.

One new P2 integration blocker remains: the new Markdown assertions check the unescaped rule name in raw report source. Current main's #735 containment correctly escapes underscores. Replaying this PR onto actual current main 0e77030 (with #704 and #709) reproduces 17 failures / 371 passes within the 388 relevant cases; the combined focused suite has 1,068 passes / 17 failures. All failures are the Markdown assertions at lines193/228; the detector assertions and other output formats pass.

Please update the assertions to compare rendered Markdown text, update with main, and rerun the relevant matrix before approval.

Comment thread tests/integration/test_ransomware_rule.py
chrisknvidia and others added 5 commits October 6, 2026 21:48
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
…filter

The ransomware negation filter dropped any pay/send verb directly after
"don't", "do not" or "never". Conditional threats such as "If you don't pay
0.5 bitcoin within 48 hours, your files will be deleted" and minimum-amount
demands such as "Never send less than 0.5 BTC to decrypt your files" were
therefore suppressed even though main reports them.

Treat the negation as advice only when it opens a clause (start of input or
after a line break or .;:!?, punctuation, allowing list/quote/emphasis markup
and a "you should/must" subject), and never when the verb continues with
"less than"/"fewer than". The search runs on the original buffer so the clause
boundary is never inferred from a clipped window edge. A typographic
apostrophe in "Don't" is recognised as well.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ransom notes are often hard-wrapped, but the payment-context strings required
spaces or tabs inside each phrase and kept the pay/send verb and the currency on
one line. Notes such as "To decrypt your files you must send\n0.5 BTC" or
"To decrypt\nyour files you must send 0.5 BTC" were therefore missed, as were
glued amounts such as "Send 0.5BTC".

In $payment_then_context, $context_then_payment and $explicit_ransom_payment:
- use \s instead of [ \t] inside the bounded phrase separators;
- allow one line break (and up to 80 more characters) between the verb and
  the currency;
- accept a digit immediately before btc/bitcoin.

Every window stays bounded and the rule compiles with the same YARA warnings
as before. Because a payment-first match can now run into a demand on the
next line, the negation check for $payment_then_context and
$explicit_ransom_payment judges only the leading verb; the later demand is
reported as its own instance and location.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Hi @chrisknvidia, thank you for the Markdown fix and for keeping every earlier ransom note covered across all four report formats!

Value and readiness: Ready once CI passes on the new head. The round-3 P2 is fixed. While re-checking against current main, I found two more cases where the branch missed ransom demands that main reports. I fixed both on the branch (details below), so the PR now clears #606 without losing those demands.

Previous findings

  • [P2] Markdown assertions (review at b19c8fc): Resolved in 136c99d. test_ransomware_rule.py renders the report with MarkdownIt before checking the rule name. With main's #735 escaping, the raw report contains ransomware\_behavior, and the assertions pass.
  • Round 2 (recall on encrypted-files notes, the negation window): Still resolved. The cipher, Dharma (Bitcoins), LockBit, "encrypted, and the only way…", XMR, "Don't forget to send…" and "Do not hesitate to pay…" notes are all CRITICAL in a real --no-llm scan.
  • #606: Still resolved. The funding sentence and the funding-steps doc, "encrypted at rest/in transit with TLS", "Never send BTC to strangers." and "Never pay the ransom in Bitcoin." are all clean.

What I changed on the branch (signed off)

  • 1ef7031 merges current main, with no conflicts.

  • 9a0fe9d [P1] Conditional threats were dropped as advice. _DIRECT_RANSOMWARE_NEGATION treated any "don't / do not / never" right before pay/send as advice. That dropped these demands, which main reports:

    • "If you don't pay 0.5 bitcoin within 48 hours, your files will be deleted."
    • "If you do not send 1 BTC to the address below, we will publish your stolen data."
    • "Never send less than 0.5 BTC to decrypt your files."

    The same applied to two-line notes built on them. A negation now counts as advice only when it opens its clause (after optional list, quote or emphasis markup, or a "you should" subject), and minimum-amount wording ("pay less than") is always a demand. The curly-apostrophe advice case and > **Never** pay… are now clean as well.

  • 9ac516a [P2] Hard-wrapped notes and glued amounts were missed. These were all missed and are reported on main:

    • "To decrypt your files you must send\n0.5 BTC…"
    • a 72-column wrap
    • "Send 0.5BTC…"

    In the decoded rule, the context phrases use bounded \s instead of [ \t], and one line break is allowed between the verb and the currency. A digit can precede btc, and the unbounded list separator is now \s{0,8}. $payment_then_context and $explicit_ransom_payment judge only their leading verb, so an advice line no longer claims a later demand's location. I checked that a negated decoy verb before a real demand does not hide it. The rule still compiles with the same warnings as before, and the encoding and CRLF endings are unchanged.

Non-blocking notes (follow-ups)

  • New, unlikely false positive: a line ending in "Send", followed by "BTC …" and a decrypt phrase within 160 characters. main also flags it.
  • Already present on main: "encrypted at rest" followed by a payment line is still flagged, because the storage filter covers only $ransom_note, not the notice inside $context_then_payment.
  • Narrowed by design since round 1: "You have to pay for decryption in Bitcoins." on its own, an XMR-only demand without an encrypted notice, and more than 80 characters between pay and bitcoin.

Verification (on 9ac516a):

  • test_ransomware_rule.py with test_static_yara.py: 442 passed. There are 34 new unit cases and 5 graph cases in each of the four formats; the new positives fail without each fix.
  • tests/nodes/analyzers: 5,715 passed.
  • Report suites: 624 passed.
  • ruff is clean.

Decision: Ready to merge once CI passes. A human maintainer needs to approve (reviewed head 9ac516a90830f420565f09ac3f10ed9eb052c6e8). This bot pushed commits to the branch, so it does not approve the PR itself.

rng1995 added a commit that referenced this pull request Oct 8, 2026
Resolve the static_yara.py conflict with #608 by keeping both sides:
main's built-in ransomware payment filter and this branch's prompt-reveal
prohibition filter are independent additions at the same anchors.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants