Skip to content

Fix GDI+ crash in FillRectangle with AntiAlias and 24bppRgb #14115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
merriemcgaw wants to merge 5 commits into
base: main
Choose a base branch
from
+401 −0
Open

Fix GDI+ crash in FillRectangle with AntiAlias and 24bppRgb #14115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c91c408
Fix GDI+ crash in FillRectangle with AntiAlias and 24bppRgb
merriemcgaw Dec 5, 2025
d0c4e1d
Add regression test for GDI+ crash in FillRectangle
merriemcgaw Dec 5, 2025
05d1d10
Address PR feedback: localize error message, simplify enum, expand tests
merriemcgaw Dec 5, 2025
528f02f
Address PR feedback: handle negative width/height in bounds check
merriemcgaw Dec 5, 2025
ff893a1
Fix GDI+ crash by clipping out-of-bounds drawing for Format24bppRgb +…
merriemcgaw Dec 6, 2025
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 src/System.Drawing.Common/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,7 @@
<data name="IconCouldNotBeExtracted" xml:space="preserve">
<value>The file path could not be opened or did not contain valid data.</value>
</data>
<data name="ArgumentException_GdiPlus_AntiAlias24bppRgbBounds" xml:space="preserve">
<value>Drawing operation exceeds the bounds of the image. This is a known GDI+ limitation with AntiAlias smoothing mode and Format24bppRgb pixel format.</value>
</data>
</root>
279 changes: 279 additions & 0 deletions src/System.Drawing.Common/src/System/Drawing/Graphics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,61 @@ public void FillRectangle(Brush brush, float x, float y, float width, float heig
{
ArgumentNullException.ThrowIfNull(brush);

// GDI+ has a bug where it writes out of bounds when using AntiAlias smoothing mode
// on a 24bppRgb bitmap, causing an AccessViolationException (or ExecutionEngineException in .NET 9+).
// We validate the parameters here to prevent the crash.
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
{
float left = x;
float right = x + width;
float top = y;
float bottom = y + height;

if (width < 0)
{
left = right;
right = x;
}

if (height < 0)
{
top = bottom;
bottom = y;
}

if (left < 0)
{
left = 0;
}

if (right > _backingImage.Width)
{
right = _backingImage.Width;
}

if (top < 0)
{
top = 0;
}

if (bottom > _backingImage.Height)
{
bottom = _backingImage.Height;
}

if (left >= right || top >= bottom)
{
// The rectangle is completely outside the image bounds.
return;
}

x = left;
y = top;
width = right - left;
height = bottom - top;
}
Comment on lines +1225 to +1278
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clipping logic is duplicated across four methods (FillRectangle, FillRectangles, and two DrawImage overloads). Consider extracting this into a private helper method to improve maintainability. For example:

private bool ShouldClipForAntiAlias24bppRgb(out int imageWidth, out int imageHeight)
{
    imageWidth = 0;
    imageHeight = 0;
    
    if (SmoothingMode != Drawing2D.SmoothingMode.AntiAlias ||
        _backingImage is not { PixelFormat: PixelFormat.Format24bppRgb })
    {
        return false;
    }
    
    imageWidth = _backingImage.Width;
    imageHeight = _backingImage.Height;
    return true;
}

private static void NormalizeRectangle(float x, float y, float width, float height, 
    out float left, out float right, out float top, out float bottom)
{
    left = x;
    right = x + width;
    top = y;
    bottom = y + height;
    
    if (width < 0)
    {
        left = right;
        right = x;
    }
    
    if (height < 0)
    {
        top = bottom;
        bottom = y;
    }
}

private static void ClipRectangle(ref float left, ref float right, ref float top, ref float bottom, 
    int maxWidth, int maxHeight)
{
    if (left < 0) left = 0;
    if (right > maxWidth) right = maxWidth;
    if (top < 0) top = 0;
    if (bottom > maxHeight) bottom = maxHeight;
}

This would reduce duplication and make the code more maintainable.

Copilot uses AI. Check for mistakes.

CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangle(
NativeGraphics,
brush.NativeBrush,
Expand Down Expand Up @@ -1261,6 +1316,92 @@ void FillRectangles(Brush brush, params ReadOnlySpan<RectangleF> rects)
{
ArgumentNullException.ThrowIfNull(brush);

if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
{
RectangleF[]? clippedRects = null;
int clippedCount = 0;

for (int i = 0; i < rects.Length; i++)
{
RectangleF rect = rects[i];
float left = rect.X;
float right = rect.X + rect.Width;
float top = rect.Y;
float bottom = rect.Y + rect.Height;

if (rect.Width < 0)
{
left = right;
right = rect.X;
}

if (rect.Height < 0)
{
top = bottom;
bottom = rect.Y;
}

if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
{
if (clippedRects is null)
{
clippedRects = new RectangleF[rects.Length];
// Copy previous valid rects
for (int j = 0; j < i; j++)
{
clippedRects[clippedCount++] = rects[j];
}
}

// Clip
if (left < 0)
{
left = 0;
}

if (right > _backingImage.Width)
{
right = _backingImage.Width;
}

if (top < 0)
{
top = 0;
}

if (bottom > _backingImage.Height)
{
bottom = _backingImage.Height;
}

if (left < right && top < bottom)
{
clippedRects[clippedCount++] = new RectangleF(left, top, right - left, bottom - top);
}
}
else if (clippedRects is not null)
{
clippedRects[clippedCount++] = rect;
}
else
{
clippedCount++;
}
}

if (clippedRects is not null)
{
fixed (RectangleF* r = clippedRects)
{
CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangles(NativeGraphics, brush.NativeBrush, (RectF*)r, clippedCount));
}

GC.KeepAlive(brush);
return;
}
}

fixed (RectangleF* r = rects)
{
CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangles(NativeGraphics, brush.NativeBrush, (RectF*)r, rects.Length));
Expand Down Expand Up @@ -2008,6 +2149,76 @@ public void DrawImage(Image image, float x, float y)
public void DrawImage(Image image, float x, float y, float width, float height)
{
ArgumentNullException.ThrowIfNull(image);

Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the coding guidelines (section 1.11), an empty line should be inserted after structure blocks. Add an empty line between ArgumentNullException.ThrowIfNull(image); and the start of the clipping logic to improve readability.

Copilot generated this review using guidance from repository custom instructions.
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
{
float left = x;
float right = x + width;
float top = y;
float bottom = y + height;

if (width < 0)
{
left = right;
right = x;
}

if (height < 0)
{
top = bottom;
bottom = y;
}

if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
{
float clippedLeft = left;
float clippedRight = right;
float clippedTop = top;
float clippedBottom = bottom;

if (clippedLeft < 0)
{
clippedLeft = 0;
}

if (clippedRight > _backingImage.Width)
{
clippedRight = _backingImage.Width;
}

if (clippedTop < 0)
{
clippedTop = 0;
}

if (clippedBottom > _backingImage.Height)
{
clippedBottom = _backingImage.Height;
}

if (clippedLeft >= clippedRight || clippedTop >= clippedBottom)
{
return;
}

float destW = right - left;
float destH = bottom - top;

if (Math.Abs(destW) > float.Epsilon && Math.Abs(destH) > float.Epsilon)
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check Math.Abs(destW) > float.Epsilon may not be the most appropriate for graphics dimensions. float.Epsilon is the smallest positive value greater than zero (approximately 1.4e-45), which is extremely small. For graphics operations, consider using a more reasonable threshold for zero comparisons, such as 0.001f or simply checking for exact zero if negative dimensions are normalized earlier. The current check would allow extremely small non-zero values that could still cause issues in division operations.

Copilot uses AI. Check for mistakes.
{
float srcX = (clippedLeft - left) / destW * image.Width;
float srcY = (clippedTop - top) / destH * image.Height;
float srcW = (clippedRight - clippedLeft) / destW * image.Width;
float srcH = (clippedBottom - clippedTop) / destH * image.Height;

DrawImage(image, new RectangleF(clippedLeft, clippedTop, clippedRight - clippedLeft, clippedBottom - clippedTop),
new RectangleF(srcX, srcY, srcW, srcH), GraphicsUnit.Pixel);
return;
}
}
}

Status status = PInvokeGdiPlus.GdipDrawImageRect(NativeGraphics, image.Pointer(), x, y, width, height);
IgnoreMetafileErrors(image, ref status);
CheckErrorStatus(status);
Expand Down Expand Up @@ -2108,6 +2319,74 @@ public void DrawImage(Image image, RectangleF destRect, RectangleF srcRect, Grap
{
ArgumentNullException.ThrowIfNull(image);

Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the coding guidelines (section 1.11), an empty line should be inserted after structure blocks. Add an empty line between ArgumentNullException.ThrowIfNull(image); and the start of the clipping logic to improve readability.

Copilot generated this review using guidance from repository custom instructions.
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
{
float left = destRect.X;
float right = destRect.X + destRect.Width;
float top = destRect.Y;
float bottom = destRect.Y + destRect.Height;

if (destRect.Width < 0)
{
left = right;
right = destRect.X;
}

if (destRect.Height < 0)
{
top = bottom;
bottom = destRect.Y;
}

if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
{
float clippedLeft = left;
float clippedRight = right;
float clippedTop = top;
float clippedBottom = bottom;

if (clippedLeft < 0)
{
clippedLeft = 0;
}

if (clippedRight > _backingImage.Width)
{
clippedRight = _backingImage.Width;
}

if (clippedTop < 0)
{
clippedTop = 0;
}

if (clippedBottom > _backingImage.Height)
{
clippedBottom = _backingImage.Height;
}

if (clippedLeft >= clippedRight || clippedTop >= clippedBottom)
{
return;
}

float destW = right - left;
float destH = bottom - top;

if (Math.Abs(destW) > float.Epsilon && Math.Abs(destH) > float.Epsilon)
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check Math.Abs(destW) > float.Epsilon may not be the most appropriate for graphics dimensions. float.Epsilon is the smallest positive value greater than zero (approximately 1.4e-45), which is extremely small. For graphics operations, consider using a more reasonable threshold for zero comparisons, such as 0.001f or simply checking for exact zero if negative dimensions are normalized earlier. The current check would allow extremely small non-zero values that could still cause issues in division operations.

Copilot uses AI. Check for mistakes.
{
float srcX = srcRect.X + (clippedLeft - left) / destW * srcRect.Width;
float srcY = srcRect.Y + (clippedTop - top) / destH * srcRect.Height;
float srcW = (clippedRight - clippedLeft) / destW * srcRect.Width;
float srcH = (clippedBottom - clippedTop) / destH * srcRect.Height;

destRect = new RectangleF(clippedLeft, clippedTop, clippedRight - clippedLeft, clippedBottom - clippedTop);
srcRect = new RectangleF(srcX, srcY, srcW, srcH);
}
}
}

Status status = PInvokeGdiPlus.GdipDrawImageRectRect(
NativeGraphics,
image.Pointer(),
Expand Down
Loading
Loading