Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -665,14 +665,15 @@ 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:
# a first-party method owning a by-value IDisposable param is a handoff (a `call`
# 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

30 changes: 30 additions & 0 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/after.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
42 changes: 42 additions & 0 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
22 changes: 22 additions & 0 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/case.own
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OWN001
64 changes: 64 additions & 0 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md
Original file line number Diff line number Diff line change
@@ -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`.
Loading