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
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,13 @@ jobs:
# #59: nameof must not masquerade as a capture/escape).
echo "$out" | grep -qE "OWN001.*'nofLeak' is never disposed" \
|| { echo "FAIL: expected OWN001 on the nameof-in-lambda local (not a capture)"; exit 1; }
# owning-factory RECALL (crypto): a System.Security.Cryptography static `Create*` factory
# returning an IDisposable is an owning acquire like File.Open*/Create* -> an undisposed
# one leaks ('rngLeak' = RandomNumberGenerator.Create()); the disposed sibling
# ('shaClean' = SHA256.Create() + Dispose) stays silent. Reduced from the SECOND,
# previously-missed leak in ShareX's DeriveCryptoData.
echo "$out" | grep -qE "OWN001.*'rngLeak' is never disposed" \
|| { echo "FAIL: expected OWN001 on the undisposed crypto-factory acquire"; exit 1; }
# dispose-optional (Task), disposed/escaping locals, a `for` loop whose
# disposable is disposed after it (`looped`, balanced), a balanced
# acquire+dispose in a loop (`whileClean`), a try/finally dispose (`tfClean`,
Expand All @@ -413,7 +420,7 @@ jobs:
# case disposes (no default) -> last case is the tail, no phantom no-match leak.
# `ncf`: `ncf?.Dispose()` (null-conditional) in a threaded finally IS a release
# (member-binding form), so it is disposed on the return path -> silent (Codex review).
for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured; do
for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured shaClean; do
if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi
done
echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, beyond flat)"
Expand Down
11 changes: 6 additions & 5 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/before.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// 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.
// Two crypto IDisposables are created on the upload path and never disposed, both now
// CAUGHT (OWN001):
// * Rfc2898DeriveBytes — the PBKDF2 deriver, which holds an HMAC (a `new` acquire);
// * RandomNumberGenerator.Create() — a static crypto FACTORY acquire (the extractor
// recognises System.Security.Cryptography `Create*` factories that return an
// IDisposable, alongside `new` and File.Open*/Create*; 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
Expand Down
17 changes: 9 additions & 8 deletions corpus/real-world/sharex-rfc2898-derivebytes-leak/notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ Vault_ooo.cs:216: error: [OWN001] IDisposable local 'rfcDeriver' is never dispos
`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)
## The second leak — also caught (crypto owning-factory)

`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.
`DeriveCryptoData` leaks **two** crypto disposables: `rfcDeriver` (acquired via `new`)
and `rng = RandomNumberGenerator.Create()` (acquired via a static **factory**). The flow
detector now catches both — `IsOwningFactory` recognises the
`System.Security.Cryptography` static `Create*` factories that return an `IDisposable`
(`RandomNumberGenerator.Create()`, `SHA256.Create()`, `Aes.Create()`, …) alongside `new`
and the `System.IO.File.Open*/Create*` factories. (Originally `rng` was a documented
recall gap; the crypto owning-factory slice closed it.) `after.cs` disposes both, so the
fix is genuinely clean.

## Files

Expand Down
56 changes: 38 additions & 18 deletions frontend/roslyn/OwnSharp.Extractor/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@
&& (v.Initializer?.Value is ObjectCreationExpressionSyntax
or ImplicitObjectCreationExpressionSyntax
|| IsPoolRent(v.Initializer?.Value, model) // ArrayPool<T> Rent
|| IsOwningFactory(v.Initializer?.Value, model))) // File.Open*/Create* factory
|| IsOwningFactory(v.Initializer?.Value, model))) // File / crypto Create* factory
nodes.Add(new { op = "acquire", var = v.Identifier.Text, line = LineOf(v) });
return true;
case ExpressionStatementSyntax es:
Expand Down Expand Up @@ -801,26 +801,46 @@

// A factory call that CREATES and hands back a fresh owned IDisposable the caller must
// release — recognised via the resolved symbol (curated, the same spirit as
// IsDisposableType is for `new`). System.IO.File.Open*/Create*/*Text return a NEW
// FileStream / StreamReader / StreamWriter that the caller owns exactly as if it had
// `new`'d one, so a local bound to one is an acquire. Curated + symbol-resolved, so a
// borrowed/cached disposable handed back by some other API is never mistaken for an
// owned acquire (precision over recall — the set grows only as ownership is certain).
// IsDisposableType is for `new`). Two families:
// * System.IO.File.Open*/Create*/*Text -> a NEW FileStream / StreamReader / StreamWriter
// the caller owns exactly as if it had `new`'d one.
// * System.Security.Cryptography static `Create*` factories -> a NEW owned IDisposable:
// RandomNumberGenerator.Create(), Aes.Create(), SHA256.Create(), RSA.Create(),
// IncrementalHash.CreateHash(), ... (guarded by static + Create-prefixed + the RESULT
// implementing IDisposable + the crypto namespace, so an instance `CreateEncryptor()` or
// a non-IDisposable `CreateFromName()` is never mistaken for one).
// Curated + symbol-resolved, so a borrowed/cached disposable handed back by some other API is
// never mistaken for an owned acquire (precision over recall — the set grows only as
// ownership is certain).
static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model)
{
if (e is not InvocationExpressionSyntax i
|| model.GetSymbolInfo(i).Symbol is not IMethodSymbol sym
|| sym.Name is not ("OpenRead" or "OpenWrite" or "Open" or "Create"
or "OpenText" or "CreateText" or "AppendText"))
|| model.GetSymbolInfo(i).Symbol is not IMethodSymbol sym)
return false;
INamedTypeSymbol? ct = sym.ContainingType;
if (ct is null || ct.Name != "File")
return false;
INamespaceSymbol? ns = ct.ContainingNamespace; // System.IO.File -> IO
if (ns is null || ns.Name != "IO")
return false;
ns = ns.ContainingNamespace; // IO -> System
return ns is { Name: "System" } && ns.ContainingNamespace is { IsGlobalNamespace: true };
if (sym.Name is "OpenRead" or "OpenWrite" or "Open" or "Create"
or "OpenText" or "CreateText" or "AppendText"
&& sym.ContainingType is { Name: "File" } ft
&& IsInNamespace(ft, "System", "IO"))
return true;
if (sym.IsStatic
&& sym.Name.StartsWith("Create", StringComparison.Ordinal)
&& ImplementsIDisposable(sym.ReturnType)
&& IsInNamespace(sym.ContainingType, "System", "Security", "Cryptography"))
return true;
return false;
}

// Is the type `t` declared in the namespace named by `parts` (outermost-first), e.g.
// IsInNamespace(t, "System", "IO") for System.IO? Walks the containing-namespace chain
// and requires it to bottom out at the global namespace (so `System.IO` matches but a
// nested `Foo.System.IO` would not).
static bool IsInNamespace(INamedTypeSymbol? t, params string[] parts)
{
var ns = t?.ContainingNamespace;
for (var k = parts.Length - 1; k >= 0; k--, ns = ns?.ContainingNamespace)
if (ns is null || ns.Name != parts[k])
return false;
return ns is { IsGlobalNamespace: true };
}

// The local names of arguments handed to a first-party CONSUMER at this call — a method
Expand Down Expand Up @@ -1081,7 +1101,7 @@
.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
.ToList();
var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 1104 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.
var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
// P-004 WPF profile: widen the reference set with assemblies named by the
// OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
Expand Down Expand Up @@ -1433,7 +1453,7 @@
candidates.Add(v.Identifier.Text);
poolBuffers.Add(v.Identifier.Text);
}
else if (IsOwningFactory(v.Initializer?.Value, model)) // File.Open*/Create* factory
else if (IsOwningFactory(v.Initializer?.Value, model)) // File / crypto Create* factory
candidates.Add(v.Identifier.Text);
}
if (candidates.Count == 0)
Expand Down
21 changes: 21 additions & 0 deletions frontend/roslyn/samples/FlowLocalsSample.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Security.Cryptography;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -11,7 +12,7 @@
// OWN002: used after Dispose()
public void UseAfterDispose()
{
var uad = new MemoryStream();

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN002

[OWN002] IDisposable local 'uad' is used after it is disposed [resource: disposable]

Check warning on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'uad' is used after it is disposed

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'uad' is used after it is disposed

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN002

[OWN002] IDisposable local 'uad' is used after it is disposed [resource: disposable]

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN002

[OWN002] IDisposable local 'uad' is used after it is disposed [resource: disposable]

Check warning on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'uad' is used after it is disposed

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'uad' is used after it is disposed

Check failure on line 15 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN002

[OWN002] IDisposable local 'uad' is used after it is disposed [resource: disposable]
uad.WriteByte(1);
uad.Dispose();
uad.WriteByte(2);
Expand All @@ -20,7 +21,7 @@
// OWN001: disposed only on the `then` path -> leaks on the else path
public void LeakOnElse(bool c)
{
var leak = new MemoryStream();

Check warning on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'leak' may not be disposed on every path (leak)

Check failure on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'leak' may not be disposed on every path (leak)

Check failure on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'leak' may not be disposed on every path (leak) [resource: disposable]

Check warning on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'leak' may not be disposed on every path (leak)

Check failure on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'leak' may not be disposed on every path (leak)

Check failure on line 24 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'leak' may not be disposed on every path (leak) [resource: disposable]
if (c)
{
leak.Dispose();
Expand All @@ -30,7 +31,7 @@
// OWN003: disposed twice
public void DoubleDispose()
{
var dbl = new MemoryStream();

Check failure on line 34 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'dbl' is disposed more than once

Check failure on line 34 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN003

[OWN003] IDisposable local 'dbl' is disposed more than once [resource: disposable]

Check failure on line 34 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'dbl' is disposed more than once

Check failure on line 34 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN003

[OWN003] IDisposable local 'dbl' is disposed more than once [resource: disposable]
dbl.Dispose();
dbl.Dispose();
}
Expand Down Expand Up @@ -60,7 +61,7 @@
{
while (n > 0)
{
var whileLeak = new MemoryStream();

Check failure on line 64 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'whileLeak' is never disposed (leak)

Check failure on line 64 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'whileLeak' is never disposed (leak) [resource: disposable]

Check failure on line 64 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'whileLeak' is never disposed (leak)

Check failure on line 64 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'whileLeak' is never disposed (leak) [resource: disposable]
whileLeak.WriteByte(1);
n = n - 1;
}
Expand All @@ -71,7 +72,7 @@
{
foreach (var it in items)
{
var foreachLeak = new MemoryStream();

Check failure on line 75 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'foreachLeak' is never disposed (leak)

Check failure on line 75 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'foreachLeak' is never disposed (leak) [resource: disposable]

Check failure on line 75 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'foreachLeak' is never disposed (leak)

Check failure on line 75 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'foreachLeak' is never disposed (leak) [resource: disposable]
foreachLeak.WriteByte((byte)it);
}
}
Expand All @@ -83,7 +84,7 @@
{
for (int i = 0; i < n; i++)
{
var forLeak = new MemoryStream();

Check failure on line 87 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'forLeak' is never disposed (leak)

Check failure on line 87 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'forLeak' is never disposed (leak) [resource: disposable]

Check failure on line 87 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

IDisposable local 'forLeak' is never disposed (leak)

Check failure on line 87 in frontend/roslyn/samples/FlowLocalsSample.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

OWN001

[OWN001] IDisposable local 'forLeak' is never disposed (leak) [resource: disposable]
forLeak.WriteByte((byte)i);
}
}
Expand Down Expand Up @@ -441,6 +442,26 @@
Action log = () => System.Console.WriteLine(nameof(nofLeak));
log();
}

// OWN001 (recall): a crypto IDisposable acquired via a static FACTORY, not `new` — the
// extractor recognises System.Security.Cryptography `Create*` factories that return an
// IDisposable as owning acquires (like File.Open*/Create*). `rngLeak` is created and never
// disposed -> leak. Reduced from the SECOND, previously-missed leak in ShareX's
// DeriveCryptoData (RandomNumberGenerator.Create()).
public void CryptoFactoryLeaks()
{
var rngLeak = RandomNumberGenerator.Create();

Check warning

Code scanning / Own.NET

owned resource not released on all paths (possible leak) Warning

IDisposable local 'rngLeak' is never disposed (leak) [resource: disposable]
rngLeak.GetBytes(new byte[8]);
}

// clean: the same kind of factory acquire, disposed on every path -> silent (it is an
// acquire exactly like `new`, so a matching Dispose balances it; not a false positive).
public void CryptoFactoryDisposed()
{
var shaClean = SHA256.Create();
shaClean.ComputeHash(new byte[1]);
shaClean.Dispose();
}
}

// A domain exception type literally named `Exception`, in a non-System namespace — the
Expand Down
Loading