From e77cc8f62fa9044f5359270c8a7e83ac4ab5d2d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 06:02:58 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(corpus):=20lock=20the=20ShareX=20Rfc28?= =?UTF-8?q?98DeriveBytes=20leak=20as=20a=20regression=20(recall=209?= =?UTF-8?q?=E2=86=9210)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-mine of ShareX on the post-#57 extractor (modeless-Form FPs gone) left a short, mostly-real list of local-disposable findings. The cleanest real bug: ShareX.UploadersLib/FileUploaders/Vault_ooo.cs:216 (the Vault.ooo uploader's DeriveCryptoData) creates an Rfc2898DeriveBytes (PBKDF2 deriver, holds an HMAC) to derive the AES key + IV, then returns without disposing it — a real resource leak on every upload. The deriver does not escape (the returned CryptoData holds only the derived byte[]s), so it is a clean local leak the flow detector catches as OWN001. It is an oversight, not a pattern: the sibling EncryptBytes() in the same file already `using`-scopes its aes/MemoryStream/CryptoStream. Captured as corpus/real-world/sharex-rfc2898-derivebytes-leak/ in the established shape: before.cs (the leak, real System.Security.Cryptography types, no ref pack), after.cs (both crypto disposables `using`-scoped → silent), case.own (the OwnLang reduction → OWN001, checked by tests/test_corpus.py), expected-diagnostics.txt, notes.md. Ratchets the benchmark floor 9→10 so the catch is pinned. notes.md also records the honest recall gap the same method exposes: it leaks a SECOND crypto disposable, rng = RandomNumberGenerator.Create(), which the extractor misses because it is a static FACTORY (the detector knows `new` and File.Open*/Create*, not arbitrary X.Create()). after.cs disposes both, so the fix is genuinely clean; the owning-factory extension is a separate recall slice. Validated locally: case.own → OWN001; tests/test_corpus.py 8/8; the modeled before.cs facts → OWN001 'is never disposed'; after.cs (using) → silent. The before.cs→facts extractor step is validated in CI (corpus-benchmark). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 2 +- .../sharex-rfc2898-derivebytes-leak/after.cs | 30 +++++++++ .../sharex-rfc2898-derivebytes-leak/before.cs | 42 ++++++++++++ .../sharex-rfc2898-derivebytes-leak/case.own | 22 +++++++ .../expected-diagnostics.txt | 1 + .../sharex-rfc2898-derivebytes-leak/notes.md | 64 +++++++++++++++++++ 6 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 corpus/real-world/sharex-rfc2898-derivebytes-leak/after.cs create mode 100644 corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs create mode 100644 corpus/real-world/sharex-rfc2898-derivebytes-leak/case.own create mode 100644 corpus/real-world/sharex-rfc2898-derivebytes-leak/expected-diagnostics.txt create mode 100644 corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0017ba1..e963128e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -674,5 +674,5 @@ jobs: # op), so a use after the handoff trips OWN002 (the cut is the signature, like # Rust's move). Remaining backlog: a cross-method use-after-dispose and an # injected-source region-escape. A drop below the floor is a regression. - run: python scripts/benchmark.py --min-recall 9 + run: python scripts/benchmark.py --min-recall 10 diff --git a/corpus/real-world/sharex-rfc2898-derivebytes-leak/after.cs b/corpus/real-world/sharex-rfc2898-derivebytes-leak/after.cs new file mode 100644 index 00000000..671e8c66 --- /dev/null +++ b/corpus/real-world/sharex-rfc2898-derivebytes-leak/after.cs @@ -0,0 +1,30 @@ +// AFTER (fixed): dispose both crypto IDisposables. The PBKDF2 deriver and the RNG are +// scoped with `using`, exactly how the sibling EncryptBytes() already handles its aes / +// MemoryStream / CryptoStream. Both are released on every path (the Key/IV are derived +// before the deriver's scope ends), so the flow detector stays silent. +using System.Security.Cryptography; + +static class VaultCrypto +{ + static CryptoData DeriveCryptoData(byte[] key) + { + byte[] salt = new byte[8]; + using var rng = RandomNumberGenerator.Create(); + rng.GetBytes(salt); + + using var rfcDeriver = new Rfc2898DeriveBytes(key, salt, 10000, HashAlgorithmName.SHA256); + return new CryptoData + { + Salt = salt, + Key = rfcDeriver.GetBytes(32), + IV = rfcDeriver.GetBytes(16), + }; + } + + class CryptoData + { + public byte[] Salt; + public byte[] Key; + public byte[] IV; + } +} diff --git a/corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs b/corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs new file mode 100644 index 00000000..ab680682 --- /dev/null +++ b/corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs @@ -0,0 +1,42 @@ +// BEFORE (buggy). Reduced from ShareX @ ed2a864 — +// ShareX.UploadersLib/FileUploaders/Vault_ooo.cs:216 (the Vault.ooo file uploader's +// DeriveCryptoData), found by mining (docs/notes/real-world-mining.md). +// +// Two crypto IDisposables are created on the upload path and never disposed: +// * Rfc2898DeriveBytes — the PBKDF2 deriver, which holds an HMAC -> CAUGHT (OWN001); +// * RandomNumberGenerator.Create() — a FACTORY-acquired IDisposable the extractor +// does not yet recognise (it knows `new` and File.Open*/Create*, not arbitrary +// `X.Create()` factories) -> a known recall gap, see notes.md. +// +// It is a genuine oversight, not a deliberate pattern: the sibling EncryptBytes() in +// the same file wraps its aes / MemoryStream / CryptoStream in `using`. The deriver +// does not escape — the returned CryptoData holds only the derived byte[]s — so it is +// a clean local leak. Wrapped in a class so the extractor's per-class flow pass visits +// it; uses the real System.Security.Cryptography types (in the BCL, no ref pack). +using System.Security.Cryptography; + +static class VaultCrypto +{ + static CryptoData DeriveCryptoData(byte[] key) + { + byte[] salt = new byte[8]; + RandomNumberGenerator rng = RandomNumberGenerator.Create(); // recall gap: factory, not yet flagged + rng.GetBytes(salt); + + Rfc2898DeriveBytes rfcDeriver = // <-- OWN001: never disposed + new Rfc2898DeriveBytes(key, salt, 10000, HashAlgorithmName.SHA256); + return new CryptoData + { + Salt = salt, + Key = rfcDeriver.GetBytes(32), // AES-256 key + IV = rfcDeriver.GetBytes(16), // AES-128 block IV + }; + } + + class CryptoData + { + public byte[] Salt; + public byte[] Key; + public byte[] IV; + } +} diff --git a/corpus/real-world/sharex-rfc2898-derivebytes-leak/case.own b/corpus/real-world/sharex-rfc2898-derivebytes-leak/case.own new file mode 100644 index 00000000..cb5b1dac --- /dev/null +++ b/corpus/real-world/sharex-rfc2898-derivebytes-leak/case.own @@ -0,0 +1,22 @@ +// OwnLang model of a real disposable leak found by mining ShareX @ ed2a864 +// (ShareX.UploadersLib/FileUploaders/Vault_ooo.cs:216, DeriveCryptoData). An +// Rfc2898DeriveBytes (the PBKDF2 deriver) is created on the upload path, used to +// derive the AES Key + IV, and never disposed — it holds an HMAC, so the leak is real. +// Modelled as a disposable acquire/release; the missing `release` is the generic +// OWN001 leak. The deriver does not escape (only the derived byte[]s are returned), +// so it stays tracked. See notes.md for provenance, the second (factory) leak the +// extractor still misses, and the honesty caveat. +module Corpus +resource Deriver { + acquire create + release dispose + kind "disposable" + emit_type "Rfc2898DeriveBytes" + emit_acquire "new Rfc2898DeriveBytes({args})" + emit_release "{0}.Dispose()" +} +fn DeriveCryptoData(key: int) { + let rfcDeriver = acquire Deriver(key); // new Rfc2898DeriveBytes(key, salt, 10000, SHA256) + // derive Key + IV via rfcDeriver.GetBytes(...) + // no `release rfcDeriver;` — the deriver is never disposed (OWN001) +} diff --git a/corpus/real-world/sharex-rfc2898-derivebytes-leak/expected-diagnostics.txt b/corpus/real-world/sharex-rfc2898-derivebytes-leak/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/sharex-rfc2898-derivebytes-leak/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md b/corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md new file mode 100644 index 00000000..cb3b5a3f --- /dev/null +++ b/corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md @@ -0,0 +1,64 @@ +# ShareX — `Rfc2898DeriveBytes` (PBKDF2 deriver) created on the upload path, never disposed + +**Found by mining** `ShareX/ShareX` @ `ed2a864` (the re-mine after the WinForms +modeless-`Form` precision fix, #57 — see `docs/notes/real-world-mining.md` and +`docs/notes/winforms-modeless-precision.md`). Once the dominant modeless-`Form` false +positives were gone, the local-disposable findings dropped to a short, mostly-real +list; this is the cleanest real bug in it. + +Location: `ShareX.UploadersLib/FileUploaders/Vault_ooo.cs:216`, the Vault.ooo file +uploader's `DeriveCryptoData`. + +## The bug + +```csharp +private static Vault_oooCryptoData DeriveCryptoData(byte[] key) +{ + byte[] salt = new byte[8]; + RandomNumberGenerator rng = RandomNumberGenerator.Create(); // leak #2 (factory) + rng.GetBytes(salt); + + Rfc2898DeriveBytes rfcDeriver = // leak #1 (new) — flagged + new Rfc2898DeriveBytes(key, salt, PBKDF2_ITERATIONS, HashAlgorithmName.SHA256); + + return new Vault_oooCryptoData { Salt = salt, + Key = rfcDeriver.GetBytes(32), IV = rfcDeriver.GetBytes(16) }; +} +``` + +`Rfc2898DeriveBytes` is `IDisposable` and holds an internal HMAC. It is created to +derive the AES key + IV, the method returns, and it is **never disposed** — a real +resource leak on every Vault.ooo upload. The returned `Vault_oooCryptoData` holds only +the derived `byte[]`s (Salt/Key/IV), **not** the deriver, so it does not escape: it is +a clean, method-local leak. The correct fix is `using var rfcDeriver = …`, which is +exactly what the sibling `EncryptBytes()` in the same file already does for its `aes` / +`MemoryStream` / `CryptoStream` — so this is an accidental oversight, not a pattern. + +## What the checker says (real extractor output, `--flow-locals`) + +```text +Vault_ooo.cs:216: error: [OWN001] IDisposable local 'rfcDeriver' is never disposed + (leak) [resource: disposable] +``` + +`acquire` is the `new Rfc2898DeriveBytes(…)`, the missing `release` is the absent +`Dispose()`. Because the deriver is never released on any path the wording is +"is never disposed" (vs the partial-path "may not be disposed on every path"). + +## The honest caveat — a second leak the extractor misses (recall gap) + +`DeriveCryptoData` actually leaks **two** crypto disposables. The extractor flags +`rfcDeriver` (acquired via `new`) but **not** `rng = RandomNumberGenerator.Create()`, +an `IDisposable` acquired via a static **factory**. The flow detector recognises `new` +and the `System.IO.File.Open*/Create*` factories (`IsOwningFactory`), but not arbitrary +`X.Create()` factories. Extending the owning-factory set to the common BCL crypto +factories (`RandomNumberGenerator.Create()`, `SHA256.Create()`, `Aes.Create()`, …) is a +separate recall slice; `after.cs` disposes both so the fix is genuinely clean. + +## Files + +- `before.cs` — the leak, reduced and self-contained (real `System.Security.Cryptography` + types; no reference pack needed). The extractor catches `rfcDeriver` (OWN001). +- `after.cs` — both crypto disposables scoped with `using` → silent. +- `case.own` — the OwnLang reduction (disposable acquire with no release → OWN001), + checked by `tests/test_corpus.py`. From a56602579db85ce78befb2cc51bbf8f187af2906 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 06:07:24 +0000 Subject: [PATCH 2/2] docs(ci): drift-proof the benchmark recall comment (CodeRabbit review on #58) The --min-recall bump to 10 left the comment saying "Now 9/11". Point the prose at the --min-recall value below (the authoritative floor, bumped per ratchet) instead of a hard-coded ratio, so the comment cannot drift on future ratchets. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e963128e..7b1f73cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -665,8 +665,9 @@ jobs: fi - name: Score the corpus on real C# # Precision is gated absolutely (every fix silent, zero false positives); - # recall is pinned at the measured floor and ratchets up as the extractor - # improves. Now 9/11 — pooled buffers ride the path-sensitive flow engine + # recall is pinned at the measured floor (the --min-recall value below, bumped + # per ratchet) and climbs as the extractor improves — pooled buffers ride the + # path-sensitive flow engine # (Rent/Return: OWN003/OWN002, pool resolved via the Roslyn SemanticModel so an # ALIASED receiver is caught), factory acquires (System.IO.File.Open*/Create*) are # recognised alongside `new`, and the inter-procedural CONSUME contract is modelled: