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
35 changes: 35 additions & 0 deletions corpus/wpf/handler-use-after-dispose-wrapped-delegate/after.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// FIXED. The handler guards on a disposed flag BEFORE touching the connection, so a late event
// bails before reaching disposed state. The subscription shape is unchanged
// (`+= new EventHandler(OnSourceChanged)`, self-owned source, no leak); only the read is guarded,
// so the field-UAF pass sees the `if (_disposed) return;` guard precede the read and stays silent.
using System;
using System.Data.SqlClient;

public sealed class SourceView : IDisposable
{
private readonly Publisher _source; // self-owned -> the subscription is not a leak
private readonly SqlConnection _conn;
private bool _disposed;

public SourceView()
{
_source = new Publisher();
_conn = new SqlConnection("Server=.;Database=Customers");
_source.Changed += new EventHandler(OnSourceChanged); // explicit delegate-creation subscription
}

private void OnSourceChanged(object sender, EventArgs e)
{
if (_disposed) return; // do not touch disposed state
_conn.ChangeDatabase("customers");
}

public void Dispose()
{
_disposed = true;
_conn.Dispose();
}
}

// Minimal in-file stand-in so the reduction is self-contained.
public sealed class Publisher { public event EventHandler Changed; }
37 changes: 37 additions & 0 deletions corpus/wpf/handler-use-after-dispose-wrapped-delegate/before.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// BUGGY — handler-use-after-dispose reached through an explicit-delegate `+=` subscription.
//
// The view owns its event source (`_source = new Publisher()`), so the subscription is NOT a
// leak (self-owned cycle, no OWN001). But the handler stays live and a late `Changed` event —
// fired after Dispose() — reads the disposed `_conn` DIRECTLY. That is a use-after-dispose
// (OWN002). The subscription is written with an explicit delegate creation
// `new EventHandler(OnSourceChanged)`: the extractor must normalize it to register `OnSourceChanged`
// as a live subscription target, else this OWN002 is missed (Codex P2 on #163).
using System;
using System.Data.SqlClient;

public sealed class SourceView : IDisposable
{
private readonly Publisher _source; // self-owned -> the subscription is not a leak
private readonly SqlConnection _conn;

public SourceView()
{
_source = new Publisher();
_conn = new SqlConnection("Server=.;Database=Customers");
_source.Changed += new EventHandler(OnSourceChanged); // explicit delegate-creation subscription
}

private void OnSourceChanged(object sender, EventArgs e)
{
// a late event: runs after Dispose() and reads the disposed connection directly
_conn.ChangeDatabase("customers"); // <-- use-after-dispose
}

public void Dispose()
{
_conn.Dispose();
}
}

// Minimal in-file stand-in so the reduction is self-contained.
public sealed class Publisher { public event EventHandler Changed; }
20 changes: 20 additions & 0 deletions corpus/wpf/handler-use-after-dispose-wrapped-delegate/case.own
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// OwnLang model of a use-after-dispose reached through a still-live event subscription written
// with an explicit delegate creation. `acquire` == the owned connection's construction,
// `release` == its Dispose(), `use` == a late `Changed` event (the subscribed handler) reading the
// connection AFTER Dispose(). Using a disposable after its release is the generic OWN002, tagged
// with the resource kind. The C# before/after carry the `+= new EventHandler(H)` syntax the
// extractor must normalize to register the handler as live; this .own reduction only carries the
// acquire/release/use logic the core reasons about.
module WpfHandlerAfterDisposeWrappedDelegate

resource Connection {
acquire Open
release Dispose
kind "disposable"
}

fn OnSourceChanged(source: int) {
let conn = acquire Connection(source);
release conn; // View.Dispose() disposes the owned connection
use conn; // a late Changed event reaches it after Dispose -> OWN002
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OWN002
20 changes: 20 additions & 0 deletions corpus/wpf/handler-use-after-dispose-wrapped-delegate/notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Handler-use-after-dispose through an explicit-delegate `+=` (OWN002 recognition)

**Pattern.** A view owns its event source (`_source = new Publisher()`), subscribes with an
explicit delegate creation `_source.Changed += new EventHandler(OnSourceChanged)`, and a late
`Changed` event — fired after `Dispose()` — reads the disposed `_conn` in the handler. Because the
source is self-owned, the subscription is **not** an OWN001 leak; the defect is the **OWN002**
use-after-dispose in the still-live handler.

**The bug (Own.NET extractor).** The handler-use-after-dispose pass keys live subscription targets
by `IsHandler(a.Right)` / `FieldName(a.Right)` on the **raw** RHS. A wrapped
`+= new EventHandler(OnSourceChanged)` fails `IsHandler`, so `OnSourceChanged` never enters the
subscribed-handler set and its disposed-field read escapes OWN002 (Codex P2 on #163) — the same
`NormalizeHandler` gap the release-matching fix closed, in a second pass. The fix applies
`NormalizeHandler` to the live-subscription keying too, so the wrapped and target-typed spellings
register consistently.

**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught** (OWN002) and
`after.cs` must be **silent**. Before the fix, `before.cs` is missed (the wrapped handler is never
tracked); after it, the OWN002 fires. `after.cs` guards the read (`if (_disposed) return;`) and is
clean either way — the source is self-owned, so there is no OWN001 to entangle the specificity check.
32 changes: 32 additions & 0 deletions corpus/wpf/subscription-explicit-delegate-release/after.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// FIXED. The single injected source is subscribed once in the ctor and released in
// Dispose. There is NO receiver rebinding — `_source` is readonly, so the one
// subscription created is the exact one torn down, and the `-=` unconditionally
// releases it (this is why the case does not lean on own-check's non-flow-sensitive
// "any -= in the class = released" model; see notes.md).
//
// own-check MUST treat this as released (silent). The subscription and the
// unsubscription name the SAME (receiver, handler) pair; the only difference is that
// `+=` wraps the handler in `new PropertyChangedEventHandler(...)` while `-=` uses a
// bare method group. An extractor that keys release on the raw handler text sees
// `new Prop...Handler(H)` != `H` and falsely reports a leak — the false positive this
// case pins.
using System;
using System.ComponentModel;

public sealed class SourceView : IDisposable
{
private readonly INotifyPropertyChanged _source; // set once in the ctor, never rebound

public SourceView(INotifyPropertyChanged source)
{
_source = source;
_source.PropertyChanged += new PropertyChangedEventHandler(OnSourcePropertyChanged); // explicit delegate
}

public void Dispose()
{
_source.PropertyChanged -= OnSourcePropertyChanged; // bare method group — releases the one subscription
}

private void OnSourcePropertyChanged(object sender, PropertyChangedEventArgs e) { /* ... */ }
}
22 changes: 22 additions & 0 deletions corpus/wpf/subscription-explicit-delegate-release/before.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// BUGGY (representative WPF/WinForms data-class pattern; hand-reduced into case.own).
//
// A view holds an injected INotifyPropertyChanged whose lifetime it does NOT own,
// subscribes to it in the ctor via an explicit delegate-creation handler, and never
// unsubscribes — no Dispose, no teardown. The injected source keeps this instance
// reachable => a real subscription leak (OWN001). This is the codebase's idiom
// (`+= new PropertyChangedEventHandler(H)`), minus the fix on `after.cs`.
using System.ComponentModel;

public sealed class SourceView
{
private readonly INotifyPropertyChanged _source; // injected, unknown lifetime

public SourceView(INotifyPropertyChanged source)
{
_source = source;
// Explicit delegate-creation handler, never released -> leak.
_source.PropertyChanged += new PropertyChangedEventHandler(OnSourcePropertyChanged);
}

private void OnSourcePropertyChanged(object sender, PropertyChangedEventArgs e) { /* ... */ }
}
24 changes: 24 additions & 0 deletions corpus/wpf/subscription-explicit-delegate-release/case.own
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
module WpfSubscriptionExplicitDelegateRelease

// A subscription token: `+= handler` acquires the source<->listener edge; the
// matching `-= handler` releases it. `kind` tags the resource so the generic
// ownership finding carries a [resource: ...] note.
resource Subscription {
acquire Subscribe
release Dispose
kind "subscription token"
}

// The view modelled as one scope: the ctor subscribes to the injected source and
// (in before.cs) never unsubscribes — no Dispose. Acquiring the token without
// releasing it = the injected source keeps the listener alive => OWN001.
//
// The C# after.cs releases it in Dispose — but writes the `-=` as a bare method
// group while
// the `+=` wraps the handler in `new PropertyChangedEventHandler(...)`. That
// syntactic asymmetry is what the extractor must normalize; this .own reduction
// only carries the acquire/release logic the core reasons about.
fn SourceView(source: int) {
let sub = acquire Subscription(source);
// no `release sub;` -> unreleased subscription (before.cs)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OWN001
62 changes: 62 additions & 0 deletions corpus/wpf/subscription-explicit-delegate-release/notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Subscription released via explicit-delegate `+=` / bare `-=` (release-match FP)

**Pattern.** A view holds an injected `INotifyPropertyChanged` it does not own,
subscribes in the ctor, and unsubscribes in `Dispose`. The codebase's idiom writes
the two sides **asymmetrically**:

```csharp
_source.PropertyChanged += new PropertyChangedEventHandler(OnSourcePropertyChanged); // ctor: explicit delegate-creation
_source.PropertyChanged -= OnSourcePropertyChanged; // Dispose: bare method group
```

Both name the same `(receiver, handler)` pair; the only difference is the `+=`
wraps the handler in a `new …Handler(...)` delegate-creation and the `-=` does not.
The explicit-delegate `+=` is the dominant subscription shape in SectorTS
(`BrokerDataClasses/*.cs`, ~250 sites).

**The bug (Own.NET extractor).** The Roslyn extractor keys release on the raw
handler *text*: `+=` records `new System.ComponentModel.PropertyChangedEventHandler(H)`
as the handler and the `-=` collector stores the bare `H`, so
`unsub.Contains("{left}|{right}")` never matches. Worse, `IsHandler` accepts only
`IdentifierName` / `MemberAccess`, so a `-= new …Handler(H)` is not even collected,
and the P-004 static-handler exemption is skipped for a `+= new …Handler(StaticM)`.
Net effect: a correctly torn-down subscription is reported as an unreleased leak —
a false positive. The fix normalizes a delegate-creation to its inner handler
before keying (extractor `NormalizeHandler`), so the wrapped `+=` and the bare `-=`
match.

**Why this case uses a single-source `Dispose` (not a rebinding setter).** The
`after.cs` here holds a **readonly** `_source` set once in the ctor and released in
`Dispose`, so "after = clean" is **unconditionally** true: the one subscription
created is the exact one torn down. It deliberately avoids the *setter-rebind*
idiom (`… -= h; _source = value; _source.PropertyChanged += new …(h)`), because
own-check's release model is **not flow-sensitive** — it treats *any* matching `-=`
in the class as releasing the subscription. Under a rebinding setter that model
would call the subscription released even though the `-=` detached only the *old*
source and the newly-assigned source is never torn down (Codex P2 on #163). That
soundness gap is **pre-existing** (a bare `-=`/`+=` rebinding setter already
behaved identically before this PR) and orthogonal to this text-normalization fix;
tracking the last-rebound subscription would need flow-sensitive release analysis,
a separate change. Keeping the corpus case rebind-free makes the regression assert
the normalization only, not the lenient model.

**What the checker says (`.own` reduction).** Modelling the ctor+Dispose as one
scope, the unreleased token in `before.cs` is the generic **OWN001** (owned
resource not released on all paths), carrying the resource-kind tag:

```text
$ python -m ownlang check corpus/wpf/subscription-explicit-delegate-release/case.own
case.own:…: error: [OWN001] 'sub' is owned but not released at end of function
[resource: subscription token]
```

**Regression guard.** `scripts/benchmark.py` runs the real C# through the
extractor + core: `before.cs` must be **caught** (the leak is real) and `after.cs`
must be **silent** (the release is recognized). Before the `NormalizeHandler` fix,
`after.cs` falsely reports OWN001 → the benchmark's specificity gate fails; after
it, silent.

**Honesty / scope.** `case.own` is a hand reduction that carries only the
acquire/release logic; the syntactic `+=`/`-=` asymmetry that the extractor must
normalize lives in `before.cs` / `after.cs`, which are representative of the real
SectorTS idiom, not a verbatim copy of one file.
28 changes: 28 additions & 0 deletions corpus/wpf/subscription-target-typed-delegate-release/after.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// FIXED. Single readonly source, subscribed in the ctor with C# 9 target-typed
// delegate creation `new(H)`, released in Dispose with a bare `-= H`. No receiver
// rebinding -> unconditionally clean.
//
// own-check MUST treat this as released (silent). The extractor has to normalize the
// target-typed delegate creation (ImplicitObjectCreationExpressionSyntax) to its inner
// handler, else `new(H)` != `H` on the release key and a correctly torn-down
// subscription is a false OWN001 (the P3 gap this case pins).
using System;
using System.ComponentModel;

public sealed class SourceView : IDisposable
{
private readonly INotifyPropertyChanged _source; // set once in the ctor, never rebound

public SourceView(INotifyPropertyChanged source)
{
_source = source;
_source.PropertyChanged += new(OnSourcePropertyChanged); // target-typed delegate creation
}

public void Dispose()
{
_source.PropertyChanged -= OnSourcePropertyChanged; // bare method group — releases the one subscription
}

private void OnSourcePropertyChanged(object sender, PropertyChangedEventArgs e) { /* ... */ }
}
18 changes: 18 additions & 0 deletions corpus/wpf/subscription-target-typed-delegate-release/before.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// BUGGY — the target-typed (C# 9) twin of subscription-explicit-delegate-release.
// The ctor subscribes an injected source using target-typed delegate creation
// `new(H)` (Roslyn: ImplicitObjectCreationExpressionSyntax) and never unsubscribes
// — no Dispose. The injected source keeps this instance reachable => leak (OWN001).
using System.ComponentModel;

public sealed class SourceView
{
private readonly INotifyPropertyChanged _source; // injected, unknown lifetime

public SourceView(INotifyPropertyChanged source)
{
_source = source;
_source.PropertyChanged += new(OnSourcePropertyChanged); // target-typed delegate creation, never released
}

private void OnSourcePropertyChanged(object sender, PropertyChangedEventArgs e) { /* ... */ }
}
22 changes: 22 additions & 0 deletions corpus/wpf/subscription-target-typed-delegate-release/case.own
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module WpfSubscriptionTargetTypedDelegateRelease

// A subscription token: `+= handler` acquires the source<->listener edge; the
// matching `-= handler` releases it. `kind` tags the resource so the finding
// carries a [resource: ...] note.
resource Subscription {
acquire Subscribe
release Dispose
kind "subscription token"
}

// The view modelled as one scope: the ctor subscribes to the injected source and
// (in before.cs) never unsubscribes — no Dispose => OWN001.
//
// The C# after.cs releases it in Dispose, but the ctor `+=` uses C# 9 target-typed
// delegate creation `new(H)` while the `-=` is a bare method group. That syntactic
// asymmetry is what the extractor must normalize; this .own reduction only carries
// the acquire/release logic the core reasons about.
fn SourceView(source: int) {
let sub = acquire Subscription(source);
// no `release sub;` -> unreleased subscription (before.cs)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OWN001
25 changes: 25 additions & 0 deletions corpus/wpf/subscription-target-typed-delegate-release/notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Subscription released via target-typed `+= new(H)` / bare `-= H` (release-match FP)

The C# 9 target-typed twin of `subscription-explicit-delegate-release`. Same FP,
different delegate-creation syntax:

```csharp
_source.PropertyChanged += new(OnSourcePropertyChanged); // ctor: target-typed delegate creation
_source.PropertyChanged -= OnSourcePropertyChanged; // Dispose: bare method group
```

**The bug (Own.NET extractor).** `NormalizeHandler` originally unwrapped only
`ObjectCreationExpressionSyntax` (explicit `new PropertyChangedEventHandler(H)`). A
target-typed `new(H)` is an `ImplicitObjectCreationExpressionSyntax`, so it was left
un-normalized: `new(H)` != `H` on the release key, and a correctly torn-down
subscription was reported as an unreleased OWN001 (Codex P3 on #163). The fix
matches `BaseObjectCreationExpressionSyntax` — the shared base of the explicit and
target-typed forms — so both normalize to their inner handler.

**Case shape.** Single **readonly** `_source`, subscribed once in the ctor and
released in `Dispose` — no receiver rebinding, so "after = clean" is unconditionally
true (same rationale as the explicit-delegate case; see its notes.md).

**Regression guard.** `scripts/benchmark.py`: `before.cs` must be caught (real
leak), `after.cs` must be silent (release recognized). Before the
`BaseObjectCreationExpressionSyntax` widening, `after.cs` falsely reports OWN001.
Loading
Loading