Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions binding/SkiaSharp/SKRegion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,12 @@ public bool Next (out SKRectI rect)

public class SpanIterator : SKObject, ISKSkipObjectRegistration
{
private readonly SKRegion region;

internal SpanIterator (SKRegion region, int y, int left, int right)
: base (SkiaApi.sk_region_spanerator_new (region.Handle, y, left, right), true)
{
this.region = region;
}

protected override void DisposeNative () =>
Expand Down
44 changes: 43 additions & 1 deletion tests/Tests/SkiaSharp/SKRegionTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Xunit;
using System;
using System.Runtime.CompilerServices;
using Xunit;

namespace SkiaSharp.Tests
{
Expand Down Expand Up @@ -281,5 +283,45 @@ public void SpanIteratorHasCorrectIntercepts()
Assert.Equal(0, left);
Assert.Equal(0, right);
}

// Regression test for the SKRegion.SpanIterator native use-after-free.
//
// The native SkRegion::Spanerator keeps a raw pointer into the region's run data, so
// the managed SpanIterator must root its SKRegion for the iterator's whole lifetime -
// exactly like the sibling RectIterator and ClipIterator already do. If it does not,
// the region can be finalized while the iterator is still alive, freeing the native
// runs the spanerator still points at -> use-after-free.
[Fact]
public void SpanIteratorKeepsItsRegionAlive()
{
var iterator = CreateDoomedSpanIterator(out var weakRegion);

// Finalize anything unrooted. Without the fix the region has no root once the
// helper returns, so it is collected here even though the iterator lives on.
CollectGarbage();

// The live iterator must keep its source region rooted (as its siblings do).
Assert.True(weakRegion.IsAlive, "The SKRegion was collected while its SpanIterator was still alive.");

// The iterator must also still be usable: reading it must not touch freed memory.
Assert.True(iterator.Next(out var left, out var right));
Assert.Equal(10, left);
Assert.Equal(110, right);

iterator.Dispose();
}
Comment thread
mattleibow marked this conversation as resolved.

[MethodImpl(MethodImplOptions.NoInlining)]
private static SKRegion.SpanIterator CreateDoomedSpanIterator(out WeakReference weakRegion)
{
// A two-rect (complex) region so the native spanerator points into real run data.
var region = new SKRegion(SKRectI.Create(10, 10, 100, 100));
region.Op(SKRectI.Create(50, 50, 100, 100), SKRegionOperation.Union);

weakRegion = new WeakReference(region);

// Only the returned iterator can keep `region` alive from here on.
return region.CreateSpanIterator(30, 5, 200);
}
}
}
Loading