Skip to content

Commit ff893a1

Browse files
merriemcgawmerriemcgaw
committed
Fix GDI+ crash by clipping out-of-bounds drawing for Format24bppRgb + AntiAlias
1 parent 528f02f commit ff893a1

File tree

2 files changed

+319
-6
lines changed

2 files changed

+319
-6
lines changed

‎src/System.Drawing.Common/src/System/Drawing/Graphics.cs‎

Lines changed: 252 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,10 +1245,36 @@ public void FillRectangle(Brush brush, float x, float y, float width, float heig
12451245
bottom = y;
12461246
}
12471247

1248-
if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
1248+
if (left < 0)
1249+
{
1250+
left = 0;
1251+
}
1252+
1253+
if (right > _backingImage.Width)
1254+
{
1255+
right = _backingImage.Width;
1256+
}
1257+
1258+
if (top < 0)
1259+
{
1260+
top = 0;
1261+
}
1262+
1263+
if (bottom > _backingImage.Height)
12491264
{
1250-
throw new ArgumentException(SR.ArgumentException_GdiPlus_AntiAlias24bppRgbBounds);
1265+
bottom = _backingImage.Height;
1266+
}
1267+
1268+
if (left >= right || top >= bottom)
1269+
{
1270+
// The rectangle is completely outside the image bounds.
1271+
return;
12511272
}
1273+
1274+
x = left;
1275+
y = top;
1276+
width = right - left;
1277+
height = bottom - top;
12521278
}
12531279

12541280
CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangle(
@@ -1290,6 +1316,92 @@ void FillRectangles(Brush brush, params ReadOnlySpan<RectangleF> rects)
12901316
{
12911317
ArgumentNullException.ThrowIfNull(brush);
12921318

1319+
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
1320+
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
1321+
{
1322+
RectangleF[]? clippedRects = null;
1323+
int clippedCount = 0;
1324+
1325+
for (int i = 0; i < rects.Length; i++)
1326+
{
1327+
RectangleF rect = rects[i];
1328+
float left = rect.X;
1329+
float right = rect.X + rect.Width;
1330+
float top = rect.Y;
1331+
float bottom = rect.Y + rect.Height;
1332+
1333+
if (rect.Width < 0)
1334+
{
1335+
left = right;
1336+
right = rect.X;
1337+
}
1338+
1339+
if (rect.Height < 0)
1340+
{
1341+
top = bottom;
1342+
bottom = rect.Y;
1343+
}
1344+
1345+
if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
1346+
{
1347+
if (clippedRects is null)
1348+
{
1349+
clippedRects = new RectangleF[rects.Length];
1350+
// Copy previous valid rects
1351+
for (int j = 0; j < i; j++)
1352+
{
1353+
clippedRects[clippedCount++] = rects[j];
1354+
}
1355+
}
1356+
1357+
// Clip
1358+
if (left < 0)
1359+
{
1360+
left = 0;
1361+
}
1362+
1363+
if (right > _backingImage.Width)
1364+
{
1365+
right = _backingImage.Width;
1366+
}
1367+
1368+
if (top < 0)
1369+
{
1370+
top = 0;
1371+
}
1372+
1373+
if (bottom > _backingImage.Height)
1374+
{
1375+
bottom = _backingImage.Height;
1376+
}
1377+
1378+
if (left < right && top < bottom)
1379+
{
1380+
clippedRects[clippedCount++] = new RectangleF(left, top, right - left, bottom - top);
1381+
}
1382+
}
1383+
else if (clippedRects is not null)
1384+
{
1385+
clippedRects[clippedCount++] = rect;
1386+
}
1387+
else
1388+
{
1389+
clippedCount++;
1390+
}
1391+
}
1392+
1393+
if (clippedRects is not null)
1394+
{
1395+
fixed (RectangleF* r = clippedRects)
1396+
{
1397+
CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangles(NativeGraphics, brush.NativeBrush, (RectF*)r, clippedCount));
1398+
}
1399+
1400+
GC.KeepAlive(brush);
1401+
return;
1402+
}
1403+
}
1404+
12931405
fixed (RectangleF* r = rects)
12941406
{
12951407
CheckErrorStatus(PInvokeGdiPlus.GdipFillRectangles(NativeGraphics, brush.NativeBrush, (RectF*)r, rects.Length));
@@ -2037,6 +2149,76 @@ public void DrawImage(Image image, float x, float y)
20372149
public void DrawImage(Image image, float x, float y, float width, float height)
20382150
{
20392151
ArgumentNullException.ThrowIfNull(image);
2152+
2153+
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
2154+
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
2155+
{
2156+
float left = x;
2157+
float right = x + width;
2158+
float top = y;
2159+
float bottom = y + height;
2160+
2161+
if (width < 0)
2162+
{
2163+
left = right;
2164+
right = x;
2165+
}
2166+
2167+
if (height < 0)
2168+
{
2169+
top = bottom;
2170+
bottom = y;
2171+
}
2172+
2173+
if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
2174+
{
2175+
float clippedLeft = left;
2176+
float clippedRight = right;
2177+
float clippedTop = top;
2178+
float clippedBottom = bottom;
2179+
2180+
if (clippedLeft < 0)
2181+
{
2182+
clippedLeft = 0;
2183+
}
2184+
2185+
if (clippedRight > _backingImage.Width)
2186+
{
2187+
clippedRight = _backingImage.Width;
2188+
}
2189+
2190+
if (clippedTop < 0)
2191+
{
2192+
clippedTop = 0;
2193+
}
2194+
2195+
if (clippedBottom > _backingImage.Height)
2196+
{
2197+
clippedBottom = _backingImage.Height;
2198+
}
2199+
2200+
if (clippedLeft >= clippedRight || clippedTop >= clippedBottom)
2201+
{
2202+
return;
2203+
}
2204+
2205+
float destW = right - left;
2206+
float destH = bottom - top;
2207+
2208+
if (Math.Abs(destW) > float.Epsilon && Math.Abs(destH) > float.Epsilon)
2209+
{
2210+
float srcX = (clippedLeft - left) / destW * image.Width;
2211+
float srcY = (clippedTop - top) / destH * image.Height;
2212+
float srcW = (clippedRight - clippedLeft) / destW * image.Width;
2213+
float srcH = (clippedBottom - clippedTop) / destH * image.Height;
2214+
2215+
DrawImage(image, new RectangleF(clippedLeft, clippedTop, clippedRight - clippedLeft, clippedBottom - clippedTop),
2216+
new RectangleF(srcX, srcY, srcW, srcH), GraphicsUnit.Pixel);
2217+
return;
2218+
}
2219+
}
2220+
}
2221+
20402222
Status status = PInvokeGdiPlus.GdipDrawImageRect(NativeGraphics, image.Pointer(), x, y, width, height);
20412223
IgnoreMetafileErrors(image, ref status);
20422224
CheckErrorStatus(status);
@@ -2137,6 +2319,74 @@ public void DrawImage(Image image, RectangleF destRect, RectangleF srcRect, Grap
21372319
{
21382320
ArgumentNullException.ThrowIfNull(image);
21392321

2322+
if (SmoothingMode == Drawing2D.SmoothingMode.AntiAlias &&
2323+
_backingImage is { PixelFormat: PixelFormat.Format24bppRgb })
2324+
{
2325+
float left = destRect.X;
2326+
float right = destRect.X + destRect.Width;
2327+
float top = destRect.Y;
2328+
float bottom = destRect.Y + destRect.Height;
2329+
2330+
if (destRect.Width < 0)
2331+
{
2332+
left = right;
2333+
right = destRect.X;
2334+
}
2335+
2336+
if (destRect.Height < 0)
2337+
{
2338+
top = bottom;
2339+
bottom = destRect.Y;
2340+
}
2341+
2342+
if (left < 0 || top < 0 || right > _backingImage.Width || bottom > _backingImage.Height)
2343+
{
2344+
float clippedLeft = left;
2345+
float clippedRight = right;
2346+
float clippedTop = top;
2347+
float clippedBottom = bottom;
2348+
2349+
if (clippedLeft < 0)
2350+
{
2351+
clippedLeft = 0;
2352+
}
2353+
2354+
if (clippedRight > _backingImage.Width)
2355+
{
2356+
clippedRight = _backingImage.Width;
2357+
}
2358+
2359+
if (clippedTop < 0)
2360+
{
2361+
clippedTop = 0;
2362+
}
2363+
2364+
if (clippedBottom > _backingImage.Height)
2365+
{
2366+
clippedBottom = _backingImage.Height;
2367+
}
2368+
2369+
if (clippedLeft >= clippedRight || clippedTop >= clippedBottom)
2370+
{
2371+
return;
2372+
}
2373+
2374+
float destW = right - left;
2375+
float destH = bottom - top;
2376+
2377+
if (Math.Abs(destW) > float.Epsilon && Math.Abs(destH) > float.Epsilon)
2378+
{
2379+
float srcX = srcRect.X + (clippedLeft - left) / destW * srcRect.Width;
2380+
float srcY = srcRect.Y + (clippedTop - top) / destH * srcRect.Height;
2381+
float srcW = (clippedRight - clippedLeft) / destW * srcRect.Width;
2382+
float srcH = (clippedBottom - clippedTop) / destH * srcRect.Height;
2383+
2384+
destRect = new RectangleF(clippedLeft, clippedTop, clippedRight - clippedLeft, clippedBottom - clippedTop);
2385+
srcRect = new RectangleF(srcX, srcY, srcW, srcH);
2386+
}
2387+
}
2388+
}
2389+
21402390
Status status = PInvokeGdiPlus.GdipDrawImageRectRect(
21412391
NativeGraphics,
21422392
image.Pointer(),

‎src/System.Drawing.Common/tests/System/Drawing/GraphicsTests.cs‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3011,17 +3011,16 @@ public void Graphics_FillRoundedRectangle_Float()
30113011
[InlineData(10, 200, 100, 100)] // Out of bounds (bottom)
30123012
[InlineData(10, 10, -100, 100)] // Out of bounds (negative width extending left)
30133013
[InlineData(10, 10, 100, -100)] // Out of bounds (negative height extending top)
3014-
public void FillRectangle_AntiAlias_24bppRgb_OutOfBounds_ThrowsArgumentException(float x, float y, float width, float height)
3014+
public void FillRectangle_AntiAlias_24bppRgb_OutOfBounds_DoesNotThrow(float x, float y, float width, float height)
30153015
{
30163016
using Bitmap bmp = new(256, 256, PixelFormat.Format24bppRgb);
30173017
using Graphics graphics = Graphics.FromImage(bmp);
30183018
graphics.SmoothingMode = SmoothingMode.AntiAlias;
30193019

30203020
// This combination causes a crash in GDI+ on .NET 9+ (ExecutionEngineException)
30213021
// and AccessViolationException on .NET 8.
3022-
// We expect our fix to throw ArgumentException instead.
3023-
Assert.Throws<ArgumentException>(() =>
3024-
graphics.FillRectangle(Brushes.Green, new RectangleF(x, y, width, height)));
3022+
// We expect our fix to clip the rectangle and not throw.
3023+
graphics.FillRectangle(Brushes.Green, new RectangleF(x, y, width, height));
30253024
}
30263025

30273026
[Theory]
@@ -3058,5 +3057,69 @@ public void FillRectangle_AntiAlias_32bppArgb_OutOfBounds_Success()
30583057
// Should not throw
30593058
graphics.FillRectangle(Brushes.Green, new RectangleF(190.5f, 180.5f, 100, 100));
30603059
}
3060+
3061+
[Fact]
3062+
public void FillRectangles_AntiAlias_24bppRgb_OutOfBounds_DoesNotThrow()
3063+
{
3064+
using Bitmap bmp = new(256, 256, PixelFormat.Format24bppRgb);
3065+
using Graphics g = Graphics.FromImage(bmp);
3066+
g.SmoothingMode = SmoothingMode.AntiAlias;
3067+
3068+
RectangleF[] rects = new[]
3069+
{
3070+
new RectangleF(190.5f, 180.5f, 100, 100), // Issue repro
3071+
new RectangleF(-100, 50, 50, 50), // Fully out left
3072+
new RectangleF(300, 50, 50, 50), // Fully out right
3073+
new RectangleF(-10, -10, 50, 50), // Partial top-left
3074+
new RectangleF(200, 200, 100, 100), // Partial bottom-right
3075+
new RectangleF(50, 50, 50, 50) // Fully inside
3076+
};
3077+
3078+
g.FillRectangles(Brushes.Green, rects);
3079+
}
3080+
3081+
[Fact]
3082+
public void DrawImage_Float_AntiAlias_24bppRgb_OutOfBounds_DoesNotThrow()
3083+
{
3084+
using Bitmap bmp = new(256, 256, PixelFormat.Format24bppRgb);
3085+
using Graphics g = Graphics.FromImage(bmp);
3086+
g.SmoothingMode = SmoothingMode.AntiAlias;
3087+
3088+
using Bitmap srcImg = new(50, 50);
3089+
using Graphics srcG = Graphics.FromImage(srcImg);
3090+
srcG.Clear(Color.Red);
3091+
3092+
// Issue repro equivalent
3093+
g.DrawImage(srcImg, 190.5f, 180.5f, 100, 100);
3094+
3095+
// Fully out of bounds
3096+
g.DrawImage(srcImg, -100, 50, 50, 50);
3097+
3098+
// Partially out of bounds
3099+
g.DrawImage(srcImg, -10, -10, 50, 50);
3100+
}
3101+
3102+
[Fact]
3103+
public void DrawImage_RectF_AntiAlias_24bppRgb_OutOfBounds_DoesNotThrow()
3104+
{
3105+
using Bitmap bmp = new(256, 256, PixelFormat.Format24bppRgb);
3106+
using Graphics g = Graphics.FromImage(bmp);
3107+
g.SmoothingMode = SmoothingMode.AntiAlias;
3108+
3109+
using Bitmap srcImg = new(50, 50);
3110+
using Graphics srcG = Graphics.FromImage(srcImg);
3111+
srcG.Clear(Color.Red);
3112+
3113+
RectangleF srcRect = new(0, 0, 50, 50);
3114+
3115+
// Issue repro equivalent
3116+
g.DrawImage(srcImg, new RectangleF(190.5f, 180.5f, 100, 100), srcRect, GraphicsUnit.Pixel);
3117+
3118+
// Fully out of bounds
3119+
g.DrawImage(srcImg, new RectangleF(-100, 50, 50, 50), srcRect, GraphicsUnit.Pixel);
3120+
3121+
// Partially out of bounds
3122+
g.DrawImage(srcImg, new RectangleF(-10, -10, 50, 50), srcRect, GraphicsUnit.Pixel);
3123+
}
30613124
#endif
30623125
}

0 commit comments

Comments
 (0)