Skip to content

Commit 41c56cb

Browse files
committed
fix(security): AnimEncoder finalizer, dim cap, frame bounds, assembly size guard
- Reject width/height outside (0, WEBP_MAX_DIMENSION] in both constructors. - New ValidateFrame helper rejects under-sized pixel buffers and bad strides before pinning and handing the pointer to libwebp. - Widen Assemble()'s webpData.size -> int truncation to a checked ulong -> int cast that throws on overflow rather than silently returning a smaller array than the actual native buffer. - Implement IDisposable properly: Dispose(bool) + ~AnimEncoder() finalizer + GC.SuppressFinalize. The finalizer path calls WebPAnimEncoderDelete directly (no FixDllNotFoundException) since finalizer threads must not enter locks that could deadlock against managed code.
1 parent 3dcc78c commit 41c56cb

1 file changed

Lines changed: 54 additions & 10 deletions

File tree

src/Imazen.WebP/AnimEncoder.cs

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ public class AnimEncoder : IDisposable
2727
/// <param name="height">Canvas height in pixels.</param>
2828
public AnimEncoder(int width, int height)
2929
{
30-
if (width <= 0) throw new ArgumentOutOfRangeException(nameof(width));
31-
if (height <= 0) throw new ArgumentOutOfRangeException(nameof(height));
30+
if (width <= 0 || width > NativeMethods.WEBP_MAX_DIMENSION)
31+
throw new ArgumentOutOfRangeException(nameof(width));
32+
if (height <= 0 || height > NativeMethods.WEBP_MAX_DIMENSION)
33+
throw new ArgumentOutOfRangeException(nameof(height));
3234

3335
_width = width;
3436
_height = height;
@@ -60,8 +62,10 @@ public AnimEncoder(int width, int height)
6062
public AnimEncoder(int width, int height, int loopCount = 0, uint backgroundColor = 0,
6163
bool allowMixed = false, bool minimizeSize = false)
6264
{
63-
if (width <= 0) throw new ArgumentOutOfRangeException(nameof(width));
64-
if (height <= 0) throw new ArgumentOutOfRangeException(nameof(height));
65+
if (width <= 0 || width > NativeMethods.WEBP_MAX_DIMENSION)
66+
throw new ArgumentOutOfRangeException(nameof(width));
67+
if (height <= 0 || height > NativeMethods.WEBP_MAX_DIMENSION)
68+
throw new ArgumentOutOfRangeException(nameof(height));
6569

6670
_width = width;
6771
_height = height;
@@ -98,6 +102,20 @@ public void AddFrame(byte[] bgraPixels, int timestampMs, float quality = -1)
98102
AddFrameInternal(bgraPixels, _width * 4, WebPPixelFormat.Bgra, timestampMs, quality);
99103
}
100104

105+
private void ValidateFrame(byte[] pixels, int stride, WebPPixelFormat format)
106+
{
107+
int bytesPerPixel = (format == WebPPixelFormat.Rgb || format == WebPPixelFormat.Bgr) ? 3 : 4;
108+
long minStride = (long)_width * bytesPerPixel;
109+
if (stride < minStride)
110+
throw new ArgumentOutOfRangeException(nameof(stride),
111+
$"stride {stride} is smaller than width*bytesPerPixel ({minStride}).");
112+
long needed = (long)stride * _height;
113+
if (pixels.Length < needed)
114+
throw new ArgumentException(
115+
$"pixels.Length {pixels.Length} is smaller than stride*height ({needed}).",
116+
nameof(pixels));
117+
}
118+
101119
/// <summary>
102120
/// Adds a frame of raw pixel data at the given timestamp.
103121
/// </summary>
@@ -120,6 +138,7 @@ public void AddFrame(byte[] pixels, int stride, WebPPixelFormat format, int time
120138
if (pixels == null) throw new ArgumentNullException(nameof(pixels));
121139
if (config == null) throw new ArgumentNullException(nameof(config));
122140
ThrowIfDisposed();
141+
ValidateFrame(pixels, stride, format);
123142

124143
if (!config.Validate())
125144
throw new ArgumentException("Invalid encoder configuration", nameof(config));
@@ -131,6 +150,7 @@ public void AddFrame(byte[] pixels, int stride, WebPPixelFormat format, int time
131150
private void AddFrameInternal(byte[] pixels, int stride, WebPPixelFormat format, int timestampMs, float quality)
132151
{
133152
ThrowIfDisposed();
153+
ValidateFrame(pixels, stride, format);
134154

135155
var config = new WebPConfig();
136156
NativeLibraryLoader.FixDllNotFoundException("webp", () =>
@@ -251,7 +271,11 @@ public byte[] Assemble()
251271
}
252272

253273
// Copy to managed array (data is owned by encoder, freed on delete)
254-
int size = (int)(ulong)webpData.size;
274+
ulong sizeU = (ulong)webpData.size;
275+
if (sizeU > int.MaxValue)
276+
throw new InvalidOperationException(
277+
$"Animation assembly produced {sizeU} bytes — exceeds int.MaxValue and cannot be returned as a byte[].");
278+
int size = (int)sizeU;
255279
byte[] output = new byte[size];
256280
Marshal.Copy(webpData.bytes, output, 0, size);
257281

@@ -275,16 +299,36 @@ private void ThrowIfDisposed()
275299

276300
public void Dispose()
277301
{
278-
if (!_disposed && _encoder != IntPtr.Zero)
302+
Dispose(true);
303+
GC.SuppressFinalize(this);
304+
}
305+
306+
~AnimEncoder()
307+
{
308+
Dispose(false);
309+
}
310+
311+
private void Dispose(bool disposing)
312+
{
313+
if (_disposed) return;
314+
315+
if (_encoder != IntPtr.Zero)
279316
{
280-
NativeLibraryLoader.FixDllNotFoundException("webpmux", () =>
317+
// Direct native call rather than via FixDllNotFoundException —
318+
// the library is already loaded (encoder handle came from it)
319+
// and finalizer threads must not call back through code that
320+
// takes locks.
321+
try
281322
{
282323
NativeMethods.WebPAnimEncoderDelete(_encoder);
283-
return 0;
284-
});
324+
}
325+
catch
326+
{
327+
// best-effort
328+
}
285329
_encoder = IntPtr.Zero;
286-
_disposed = true;
287330
}
331+
_disposed = true;
288332
}
289333
}
290334
}

0 commit comments

Comments
 (0)