Skip to content

Commit a356848

Browse files
brianpopowbrianpopow
authored
Merge pull request #1898 from SixLabors/bp/png_idatcrc
PNG: Verify CRC of IDAT chunk is correct
2 parents 5e060ba + 112585c commit a356848

File tree

4 files changed

+36
-11
lines changed

4 files changed

+36
-11
lines changed

‎src/ImageSharp/Formats/Png/PngDecoderCore.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,23 +1206,30 @@ private bool TryReadChunk(out PngChunk chunk)
12061206

12071207
PngChunkType type = this.ReadChunkType();
12081208

1209-
// NOTE: Reading the Data chunk is the responsible of the caller
12101209
// If we're reading color metadata only we're only interested in the IHDR and tRNS chunks.
12111210
// We can skip all other chunk data in the stream for better performance.
1212-
if (type == PngChunkType.Data || (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency))
1211+
if (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency)
12131212
{
12141213
chunk = new PngChunk(length, type);
12151214

12161215
return true;
12171216
}
12181217

1218+
long pos = this.currentStream.Position;
12191219
chunk = new PngChunk(
12201220
length: length,
12211221
type: type,
12221222
data: this.ReadChunkData(length));
12231223

12241224
this.ValidateChunk(chunk);
12251225

1226+
// Restore the stream position for IDAT chunks, because it will be decoded later and
1227+
// was only read to verifying the CRC is correct.
1228+
if (type == PngChunkType.Data)
1229+
{
1230+
this.currentStream.Position = pos;
1231+
}
1232+
12261233
return true;
12271234
}
12281235

‎tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ public void Identify(string imagePath, int expectedPixelSize)
255255
public void Decode_MissingDataChunk_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
256256
where TPixel : unmanaged, IPixel<TPixel>
257257
{
258-
System.Exception ex = Record.Exception(
258+
Exception ex = Record.Exception(
259259
() =>
260260
{
261261
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -271,7 +271,7 @@ public void Decode_MissingDataChunk_ThrowsException<TPixel>(TestImageProvider<TP
271271
public void Decode_InvalidBitDepth_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
272272
where TPixel : unmanaged, IPixel<TPixel>
273273
{
274-
System.Exception ex = Record.Exception(
274+
Exception ex = Record.Exception(
275275
() =>
276276
{
277277
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -287,7 +287,7 @@ public void Decode_InvalidBitDepth_ThrowsException<TPixel>(TestImageProvider<TPi
287287
public void Decode_InvalidColorType_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
288288
where TPixel : unmanaged, IPixel<TPixel>
289289
{
290-
System.Exception ex = Record.Exception(
290+
Exception ex = Record.Exception(
291291
() =>
292292
{
293293
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -297,13 +297,27 @@ public void Decode_InvalidColorType_ThrowsException<TPixel>(TestImageProvider<TP
297297
Assert.Contains("Invalid or unsupported color type", ex.Message);
298298
}
299299

300+
[Theory]
301+
[WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32)]
302+
public void Decode_InvalidDataChunkCrc_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
303+
where TPixel : unmanaged, IPixel<TPixel>
304+
{
305+
InvalidImageContentException ex = Assert.Throws<InvalidImageContentException>(
306+
() =>
307+
{
308+
using Image<TPixel> image = provider.GetImage(PngDecoder);
309+
});
310+
Assert.NotNull(ex);
311+
Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message);
312+
}
313+
300314
// https://github.com/SixLabors/ImageSharp/issues/1014
301315
[Theory]
302316
[WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)]
303317
public void Issue1014_DataSplitOverMultipleIDatChunks<TPixel>(TestImageProvider<TPixel> provider)
304318
where TPixel : unmanaged, IPixel<TPixel>
305319
{
306-
System.Exception ex = Record.Exception(
320+
Exception ex = Record.Exception(
307321
() =>
308322
{
309323
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -319,7 +333,7 @@ public void Issue1014_DataSplitOverMultipleIDatChunks<TPixel>(TestImageProvider<
319333
public void Issue1177_CRC_Omitted<TPixel>(TestImageProvider<TPixel> provider)
320334
where TPixel : unmanaged, IPixel<TPixel>
321335
{
322-
System.Exception ex = Record.Exception(
336+
Exception ex = Record.Exception(
323337
() =>
324338
{
325339
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -335,7 +349,7 @@ public void Issue1177_CRC_Omitted<TPixel>(TestImageProvider<TPixel> provider)
335349
public void Issue1127<TPixel>(TestImageProvider<TPixel> provider)
336350
where TPixel : unmanaged, IPixel<TPixel>
337351
{
338-
System.Exception ex = Record.Exception(
352+
Exception ex = Record.Exception(
339353
() =>
340354
{
341355
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -351,7 +365,7 @@ public void Issue1127<TPixel>(TestImageProvider<TPixel> provider)
351365
public void Issue1047<TPixel>(TestImageProvider<TPixel> provider)
352366
where TPixel : unmanaged, IPixel<TPixel>
353367
{
354-
System.Exception ex = Record.Exception(
368+
Exception ex = Record.Exception(
355369
() =>
356370
{
357371
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -372,7 +386,7 @@ public void Issue1047<TPixel>(TestImageProvider<TPixel> provider)
372386
public void Issue1765<TPixel>(TestImageProvider<TPixel> provider)
373387
where TPixel : unmanaged, IPixel<TPixel>
374388
{
375-
System.Exception ex = Record.Exception(
389+
Exception ex = Record.Exception(
376390
() =>
377391
{
378392
using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -388,7 +402,7 @@ public void Issue1765<TPixel>(TestImageProvider<TPixel> provider)
388402
public void Issue410_MalformedApplePng<TPixel>(TestImageProvider<TPixel> provider)
389403
where TPixel : unmanaged, IPixel<TPixel>
390404
{
391-
System.Exception ex = Record.Exception(
405+
Exception ex = Record.Exception(
392406
() =>
393407
{
394408
using Image<TPixel> image = provider.GetImage(PngDecoder);

‎tests/ImageSharp.Tests/TestImages.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ public static class Png
120120
public static class Bad
121121
{
122122
public const string MissingDataChunk = "Png/xdtn0g01.png";
123+
public const string WrongCrcDataChunk = "Png/xcsn0g01.png";
123124
public const string CorruptedChunk = "Png/big-corrupted-chunk.png";
124125

125126
// Zlib errors.

‎tests/Images/Input/Png/xcsn0g01.png‎

Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)