Merge pull request SixLabors#2824 from SixLabors/js/backport-2780

JimBobSquarePants · web-flow · commit e69a507868d1 · 2024-10-16T19:31:23.000+10:00
Backport : V3 Fix SixLabors#2779 buffer overrun
diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs
@@ -50,7 +50,7 @@ public static Image<TPixel> WrapMemory<TPixel>(
     {
         Guard.NotNull(configuration, nameof(configuration));
         Guard.NotNull(metadata, nameof(metadata));
-        Guard.IsTrue(pixelMemory.Length >= width * height, nameof(pixelMemory), "The length of the input memory is less than the specified image size");
+        Guard.IsTrue(pixelMemory.Length >= (long)width * height, nameof(pixelMemory), "The length of the input memory is less than the specified image size");
 
         MemoryGroup<TPixel> memorySource = MemoryGroup<TPixel>.Wrap(pixelMemory);
         return new Image<TPixel>(configuration, memorySource, width, height, metadata);
@@ -145,7 +145,7 @@ public static Image<TPixel> WrapMemory<TPixel>(
     {
         Guard.NotNull(configuration, nameof(configuration));
         Guard.NotNull(metadata, nameof(metadata));
-        Guard.IsTrue(pixelMemoryOwner.Memory.Length >= width * height, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size");
+        Guard.IsTrue(pixelMemoryOwner.Memory.Length >= (long)width * height, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size");
 
         MemoryGroup<TPixel> memorySource = MemoryGroup<TPixel>.Wrap(pixelMemoryOwner);
         return new Image<TPixel>(configuration, memorySource, width, height, metadata);
@@ -232,7 +232,7 @@ public static Image<TPixel> WrapMemory<TPixel>(
 
         ByteMemoryManager<TPixel> memoryManager = new(byteMemory);
 
-        Guard.IsTrue(memoryManager.Memory.Length >= width * height, nameof(byteMemory), "The length of the input memory is less than the specified image size");
+        Guard.IsTrue(memoryManager.Memory.Length >= (long)width * height, nameof(byteMemory), "The length of the input memory is less than the specified image size");
 
         MemoryGroup<TPixel> memorySource = MemoryGroup<TPixel>.Wrap(memoryManager.Memory);
         return new Image<TPixel>(configuration, memorySource, width, height, metadata);
@@ -422,10 +422,11 @@ public static unsafe Image<TPixel> WrapMemory<TPixel>(
         Guard.IsFalse(pointer == null, nameof(pointer), "Pointer must be not null");
         Guard.NotNull(configuration, nameof(configuration));
         Guard.NotNull(metadata, nameof(metadata));
+        Guard.MustBeLessThanOrEqualTo(height * (long)width, int.MaxValue, "Total amount of pixels exceeds int.MaxValue");
 
         UnmanagedMemoryManager<TPixel> memoryManager = new(pointer, width * height);
 
-        Guard.MustBeGreaterThanOrEqualTo(bufferSizeInBytes, memoryManager.Memory.Span.Length, nameof(bufferSizeInBytes));
+        Guard.MustBeGreaterThanOrEqualTo(bufferSizeInBytes / sizeof(TPixel), memoryManager.Memory.Span.Length, nameof(bufferSizeInBytes));
 
         MemoryGroup<TPixel> memorySource = MemoryGroup<TPixel>.Wrap(memoryManager.Memory);
         return new Image<TPixel>(configuration, memorySource, width, height, metadata);
diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs
@@ -294,8 +294,11 @@ public void WrapSystemDrawingBitmap_FromBytes_WhenObserved()
             }
         }
 
-        [Fact]
-        public unsafe void WrapMemory_Throws_OnTooLessWrongSize()
+        [Theory]
+        [InlineData(20, 5, 5)]
+        [InlineData(1023, 32, 32)]
+        [InlineData(65536, 65537, 65536)]
+        public unsafe void WrapMemory_Throws_OnTooLessWrongSize(int size, int width, int height)
         {
             var cfg = Configuration.CreateDefaultInstance();
             var metaData = new ImageMetadata();
@@ -306,7 +309,7 @@ public unsafe void WrapMemory_Throws_OnTooLessWrongSize()
             {
                 try
                 {
-                    using var image = Image.WrapMemory<Rgba32>(cfg, ptr, 24, 5, 5, metaData);
+                    using var image = Image.WrapMemory<Rgba32>(cfg, ptr, size * sizeof(Rgba32), width, height, metaData);
                 }
                 catch (Exception e)
                 {
@@ -317,24 +320,30 @@ public unsafe void WrapMemory_Throws_OnTooLessWrongSize()
             Assert.IsType<ArgumentOutOfRangeException>(thrownException);
         }
 
-        [Fact]
-        public unsafe void WrapMemory_FromPointer_CreatedImageIsCorrect()
+        [Theory]
+        [InlineData(25, 5, 5)]
+        [InlineData(26, 5, 5)]
+        [InlineData(2, 1, 1)]
+        [InlineData(1024, 32, 32)]
+        [InlineData(2048, 32, 32)]
+        public unsafe void WrapMemory_FromPointer_CreatedImageIsCorrect(int size, int width, int height)
         {
             var cfg = Configuration.CreateDefaultInstance();
             var metaData = new ImageMetadata();
 
-            var array = new Rgba32[25];
+            var array = new Rgba32[size];
 
             fixed (void* ptr = array)
             {
-                using (var image = Image.WrapMemory<Rgba32>(cfg, ptr, 25, 5, 5, metaData))
+                using (var image = Image.WrapMemory<Rgba32>(cfg, ptr, size * sizeof(Rgba32), width, height, metaData))
                 {
                     Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory<Rgba32> imageMem));
                     Span<Rgba32> imageSpan = imageMem.Span;
+                    Span<Rgba32> sourceSpan = array.AsSpan(0, width * height);
                     ref Rgba32 pixel0 = ref imageSpan[0];
-                    Assert.True(Unsafe.AreSame(ref array[0], ref pixel0));
+                    Assert.True(Unsafe.AreSame(ref sourceSpan[0], ref pixel0));
                     ref Rgba32 pixel_1 = ref imageSpan[imageSpan.Length - 1];
-                    Assert.True(Unsafe.AreSame(ref array[array.Length - 1], ref pixel_1));
+                    Assert.True(Unsafe.AreSame(ref sourceSpan[sourceSpan.Length - 1], ref pixel_1));
 
                     Assert.Equal(cfg, image.Configuration);
                     Assert.Equal(metaData, image.Metadata);
@@ -395,6 +404,7 @@ public unsafe void WrapSystemDrawingBitmap_FromPointer()
         [InlineData(0, 5, 5)]
         [InlineData(20, 5, 5)]
         [InlineData(1023, 32, 32)]
+        [InlineData(65536, 65537, 65536)]
         public void WrapMemory_MemoryOfT_InvalidSize(int size, int height, int width)
         {
             var array = new Rgba32[size];
@@ -430,6 +440,7 @@ private class TestMemoryOwner<T> : IMemoryOwner<T>
         [InlineData(0, 5, 5)]
         [InlineData(20, 5, 5)]
         [InlineData(1023, 32, 32)]
+        [InlineData(65536, 65537, 65536)]
         public void WrapMemory_IMemoryOwnerOfT_InvalidSize(int size, int height, int width)
         {
             var array = new Rgba32[size];
@@ -476,6 +487,7 @@ public void WrapMemory_IMemoryOwnerOfT_ValidSize(int size, int height, int width
         [InlineData(0, 5, 5)]
         [InlineData(20, 5, 5)]
         [InlineData(1023, 32, 32)]
+        [InlineData(65536, 65537, 65536)]
         public void WrapMemory_IMemoryOwnerOfByte_InvalidSize(int size, int height, int width)
         {
             var array = new byte[size * Unsafe.SizeOf<Rgba32>()];

Original file line number	Diff line number	Diff line change
`@@ -294,8 +294,11 @@ public void WrapSystemDrawingBitmap_FromBytes_WhenObserved()`
`294`	`294`	`}`
`295`	`295`	`}`
`296`	`296`
`297`		`- [Fact]`
`298`		`- public unsafe void WrapMemory_Throws_OnTooLessWrongSize()`
	`297`	`+ [Theory]`
	`298`	`+ [InlineData(20, 5, 5)]`
	`299`	`+ [InlineData(1023, 32, 32)]`
	`300`	`+ [InlineData(65536, 65537, 65536)]`
	`301`	`+ public unsafe void WrapMemory_Throws_OnTooLessWrongSize(int size, int width, int height)`
`299`	`302`	`{`
`300`	`303`	`var cfg = Configuration.CreateDefaultInstance();`
`301`	`304`	`var metaData = new ImageMetadata();`
`@@ -306,7 +309,7 @@ public unsafe void WrapMemory_Throws_OnTooLessWrongSize()`
`306`	`309`	`{`
`307`	`310`	`try`
`308`	`311`	`{`
`309`		`- using var image = Image.WrapMemory<Rgba32>(cfg, ptr, 24, 5, 5, metaData);`
	`312`	`+ using var image = Image.WrapMemory<Rgba32>(cfg, ptr, size * sizeof(Rgba32), width, height, metaData);`
`310`	`313`	`}`
`311`	`314`	`catch (Exception e)`
`312`	`315`	`{`
`@@ -317,24 +320,30 @@ public unsafe void WrapMemory_Throws_OnTooLessWrongSize()`
`317`	`320`	`Assert.IsType<ArgumentOutOfRangeException>(thrownException);`
`318`	`321`	`}`
`319`	`322`
`320`		`- [Fact]`
`321`		`- public unsafe void WrapMemory_FromPointer_CreatedImageIsCorrect()`
	`323`	`+ [Theory]`
	`324`	`+ [InlineData(25, 5, 5)]`
	`325`	`+ [InlineData(26, 5, 5)]`
	`326`	`+ [InlineData(2, 1, 1)]`
	`327`	`+ [InlineData(1024, 32, 32)]`
	`328`	`+ [InlineData(2048, 32, 32)]`
	`329`	`+ public unsafe void WrapMemory_FromPointer_CreatedImageIsCorrect(int size, int width, int height)`
`322`	`330`	`{`
`323`	`331`	`var cfg = Configuration.CreateDefaultInstance();`
`324`	`332`	`var metaData = new ImageMetadata();`
`325`	`333`
`326`		`- var array = new Rgba32[25];`
	`334`	`+ var array = new Rgba32[size];`
`327`	`335`
`328`	`336`	`fixed (void* ptr = array)`
`329`	`337`	`{`
`330`		`- using (var image = Image.WrapMemory<Rgba32>(cfg, ptr, 25, 5, 5, metaData))`
	`338`	`+ using (var image = Image.WrapMemory<Rgba32>(cfg, ptr, size * sizeof(Rgba32), width, height, metaData))`
`331`	`339`	`{`
`332`	`340`	`Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory<Rgba32> imageMem));`
`333`	`341`	`Span<Rgba32> imageSpan = imageMem.Span;`
	`342`	`+ Span<Rgba32> sourceSpan = array.AsSpan(0, width * height);`
`334`	`343`	`ref Rgba32 pixel0 = ref imageSpan[0];`
`335`		`- Assert.True(Unsafe.AreSame(ref array[0], ref pixel0));`
	`344`	`+ Assert.True(Unsafe.AreSame(ref sourceSpan[0], ref pixel0));`
`336`	`345`	`ref Rgba32 pixel_1 = ref imageSpan[imageSpan.Length - 1];`
`337`		`- Assert.True(Unsafe.AreSame(ref array[array.Length - 1], ref pixel_1));`
	`346`	`+ Assert.True(Unsafe.AreSame(ref sourceSpan[sourceSpan.Length - 1], ref pixel_1));`
`338`	`347`
`339`	`348`	`Assert.Equal(cfg, image.Configuration);`
`340`	`349`	`Assert.Equal(metaData, image.Metadata);`
`@@ -395,6 +404,7 @@ public unsafe void WrapSystemDrawingBitmap_FromPointer()`
`395`	`404`	`[InlineData(0, 5, 5)]`
`396`	`405`	`[InlineData(20, 5, 5)]`
`397`	`406`	`[InlineData(1023, 32, 32)]`
	`407`	`+ [InlineData(65536, 65537, 65536)]`
`398`	`408`	`public void WrapMemory_MemoryOfT_InvalidSize(int size, int height, int width)`
`399`	`409`	`{`
`400`	`410`	`var array = new Rgba32[size];`
`@@ -430,6 +440,7 @@ private class TestMemoryOwner<T> : IMemoryOwner<T>`
`430`	`440`	`[InlineData(0, 5, 5)]`
`431`	`441`	`[InlineData(20, 5, 5)]`
`432`	`442`	`[InlineData(1023, 32, 32)]`
	`443`	`+ [InlineData(65536, 65537, 65536)]`
`433`	`444`	`public void WrapMemory_IMemoryOwnerOfT_InvalidSize(int size, int height, int width)`
`434`	`445`	`{`
`435`	`446`	`var array = new Rgba32[size];`
`@@ -476,6 +487,7 @@ public void WrapMemory_IMemoryOwnerOfT_ValidSize(int size, int height, int width`
`476`	`487`	`[InlineData(0, 5, 5)]`
`477`	`488`	`[InlineData(20, 5, 5)]`
`478`	`489`	`[InlineData(1023, 32, 32)]`
	`490`	`+ [InlineData(65536, 65537, 65536)]`
`479`	`491`	`public void WrapMemory_IMemoryOwnerOfByte_InvalidSize(int size, int height, int width)`
`480`	`492`	`{`
`481`	`493`	`var array = new byte[size * Unsafe.SizeOf<Rgba32>()];`