diff --git a/Directory.Build.props b/Directory.Build.props
index 6e3cc9b3b..bf004921e 100644
--- a/Directory.Build.props
+++ b/Directory.Build.props
@@ -27,9 +27,12 @@
true
false
true
- true
+
+ true
+
+
false
diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs
index 72bfa3864..dd5160414 100644
--- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs
+++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
// Licensed under the Apache License, Version 2.0.
using System.Runtime.CompilerServices;
@@ -19,8 +19,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
// The number of valid bits left to read in the buffer.
private int remain;
- // Whether there is more data to pull from the stream for the current mcu.
- private bool noMore;
+ // Whether there is no more good data to pull from the stream for the current mcu.
+ private bool badData;
public HuffmanScanBuffer(DoubleBufferedStreamReader stream)
{
@@ -30,29 +30,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
this.Marker = JpegConstants.Markers.XFF;
this.MarkerPosition = 0;
this.BadMarker = false;
- this.noMore = false;
- this.Eof = false;
+ this.badData = false;
+ this.NoData = false;
}
///
- /// Gets or sets the current, if any, marker in the input stream.
+ /// Gets the current, if any, marker in the input stream.
///
- public byte Marker { get; set; }
+ public byte Marker { get; private set; }
///
- /// Gets or sets the opening position of an identified marker.
+ /// Gets the opening position of an identified marker.
///
- public long MarkerPosition { get; set; }
+ public long MarkerPosition { get; private set; }
///
- /// Gets or sets a value indicating whether we have a bad marker, I.E. One that is not between RST0 and RST7
+ /// Gets a value indicating whether a bad marker has been detected, I.E. One that is not between RST0 and RST7
///
- public bool BadMarker { get; set; }
+ public bool BadMarker { get; private set; }
///
- /// Gets or sets a value indicating whether we have prematurely reached the end of the file.
+ /// Gets a value indicating whether to continue reading the input stream.
///
- public bool Eof { get; set; }
+ public bool NoData { get; private set; }
[MethodImpl(InliningOptions.ShortMethod)]
public void CheckBits()
@@ -71,8 +71,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
this.Marker = JpegConstants.Markers.XFF;
this.MarkerPosition = 0;
this.BadMarker = false;
- this.noMore = false;
- this.Eof = false;
+ this.badData = false;
+ this.NoData = false;
}
[MethodImpl(InliningOptions.ShortMethod)]
@@ -141,39 +141,28 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
ulong temp = 0;
for (int i = 0; i < 6; i++)
{
- int b = this.noMore ? 0 : this.stream.ReadByte();
-
- if (b == -1)
- {
- // We've encountered the end of the file stream which means there's no EOI marker in the image
- // or the SOS marker has the wrong dimensions set.
- this.Eof = true;
- b = 0;
- }
+ int b = this.ReadStream();
// Found a marker.
if (b == JpegConstants.Markers.XFF)
{
- this.MarkerPosition = this.stream.Position - 1;
- int c = this.stream.ReadByte();
+ int c = this.ReadStream();
while (c == JpegConstants.Markers.XFF)
{
- c = this.stream.ReadByte();
-
- if (c == -1)
- {
- this.Eof = true;
- c = 0;
- break;
- }
+ // Loop here to discard any padding FF's on terminating marker,
+ // so that we can save a valid marker value.
+ c = this.ReadStream();
}
+ // We accept multiple FF's followed by a 0 as meaning a single FF data byte.
+ // This data pattern is not valid according to the standard.
if (c != 0)
{
this.Marker = (byte)c;
- this.noMore = true;
+ this.badData = true;
if (!this.HasRestart())
{
+ this.MarkerPosition = this.stream.Position - 2;
this.BadMarker = true;
}
}
@@ -184,5 +173,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
return temp;
}
+
+ [MethodImpl(InliningOptions.ShortMethod)]
+ private int ReadStream()
+ {
+ int value = this.badData ? 0 : this.stream.ReadByte();
+ if (value == -1)
+ {
+ // We've encountered the end of the file stream which means there's no EOI marker
+ // in the image or the SOS marker has the wrong dimensions set.
+ this.badData = true;
+ this.NoData = true;
+ value = 0;
+ }
+
+ return value;
+ }
}
-}
\ No newline at end of file
+}
diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs
index 76fea9297..c28733250 100644
--- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs
+++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
// Licensed under the Apache License, Version 2.0.
using System;
@@ -130,6 +130,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
int mcu = 0;
int mcusPerColumn = this.frame.McusPerColumn;
int mcusPerLine = this.frame.McusPerLine;
+ ref HuffmanScanBuffer buffer = ref this.scanBuffer;
// Pre-derive the huffman table to avoid in-loop checks.
for (int i = 0; i < this.componentsLength; i++)
@@ -171,6 +172,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
for (int x = 0; x < h; x++)
{
+ if (buffer.NoData)
+ {
+ return;
+ }
+
int blockCol = (mcuCol * h) + x;
this.DecodeBlockBaseline(
@@ -193,6 +199,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
private unsafe void ParseBaselineDataNonInterleaved()
{
JpegComponent component = this.components[this.frame.ComponentOrder[0]];
+ ref HuffmanScanBuffer buffer = ref this.scanBuffer;
int w = component.WidthInBlocks;
int h = component.HeightInBlocks;
@@ -210,6 +217,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
for (int i = 0; i < w; i++)
{
+ if (buffer.NoData)
+ {
+ return;
+ }
+
this.DecodeBlockBaseline(
component,
ref Unsafe.Add(ref blockRef, i),
@@ -294,6 +306,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
int mcu = 0;
int mcusPerColumn = this.frame.McusPerColumn;
int mcusPerLine = this.frame.McusPerLine;
+ ref HuffmanScanBuffer buffer = ref this.scanBuffer;
// Pre-derive the huffman table to avoid in-loop checks.
for (int k = 0; k < this.componentsLength; k++)
@@ -316,7 +329,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
int order = this.frame.ComponentOrder[k];
JpegComponent component = this.components[order];
ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId];
- ref HuffmanScanBuffer buffer = ref this.scanBuffer;
int h = component.HorizontalSamplingFactor;
int v = component.VerticalSamplingFactor;
@@ -331,7 +343,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
for (int x = 0; x < h; x++)
{
- if (buffer.Eof)
+ if (buffer.NoData)
{
return;
}
@@ -375,7 +387,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
for (int i = 0; i < w; i++)
{
- if (buffer.Eof)
+ if (buffer.NoData)
{
return;
}
@@ -404,7 +416,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
for (int i = 0; i < w; i++)
{
- if (buffer.Eof)
+ if (buffer.NoData)
{
return;
}
@@ -691,4 +703,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
return false;
}
}
-}
\ No newline at end of file
+}
diff --git a/src/ImageSharp/Formats/Jpeg/Components/ZigZag.cs b/src/ImageSharp/Formats/Jpeg/Components/ZigZag.cs
index a3701f2c1..059e2052b 100644
--- a/src/ImageSharp/Formats/Jpeg/Components/ZigZag.cs
+++ b/src/ImageSharp/Formats/Jpeg/Components/ZigZag.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
// Licensed under the Apache License, Version 2.0.
using System;
@@ -16,10 +16,22 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
[StructLayout(LayoutKind.Sequential)]
internal unsafe struct ZigZag
{
+ ///
+ /// When reading corrupted data, the Huffman decoders could attempt
+ /// to reference an entry beyond the end of this array (if the decoded
+ /// zero run length reaches past the end of the block). To prevent
+ /// wild stores without adding an inner-loop test, we put some extra
+ /// "63"s after the real entries. This will cause the extra coefficient
+ /// to be stored in location 63 of the block, not somewhere random.
+ /// The worst case would be a run-length of 15, which means we need 16
+ /// fake entries.
+ ///
+ private const int Size = 64 + 16;
+
///
/// Copy of in a value type
///
- public fixed byte Data[64];
+ public fixed byte Data[Size];
///
/// Unzig maps from the zigzag ordering to the natural ordering. For example,
@@ -27,22 +39,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
/// value is 16, which means first column (16%8 == 0) and third row (16/8 == 2).
///
private static readonly byte[] Unzig =
+ new byte[Size]
{
- 0,
- 1, 8,
- 16, 9, 2,
- 3, 10, 17, 24,
- 32, 25, 18, 11, 4,
- 5, 12, 19, 26, 33, 40,
- 48, 41, 34, 27, 20, 13, 6,
- 7, 14, 21, 28, 35, 42, 49, 56,
- 57, 50, 43, 36, 29, 22, 15,
- 23, 30, 37, 44, 51, 58,
- 59, 52, 45, 38, 31,
- 39, 46, 53, 60,
- 61, 54, 47,
- 55, 62,
- 63
+ 0, 1, 8, 16, 9, 2, 3, 10,
+ 17, 24, 32, 25, 18, 11, 4, 5,
+ 12, 19, 26, 33, 40, 48, 41, 34,
+ 27, 20, 13, 6, 7, 14, 21, 28,
+ 35, 42, 49, 56, 57, 50, 43, 36,
+ 29, 22, 15, 23, 30, 37, 44, 51,
+ 58, 59, 52, 45, 38, 31, 39, 46,
+ 53, 60, 61, 54, 47, 55, 62, 63,
+ 63, 63, 63, 63, 63, 63, 63, 63, // Extra entries for safety in decoder
+ 63, 63, 63, 63, 63, 63, 63, 63
};
///
@@ -68,7 +76,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
{
ZigZag result = default;
byte* unzigPtr = result.Data;
- Marshal.Copy(Unzig, 0, (IntPtr)unzigPtr, 64);
+ Marshal.Copy(Unzig, 0, (IntPtr)unzigPtr, Size);
return result;
}
@@ -79,7 +87,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
{
Block8x8F result = default;
- for (int i = 0; i < 64; i++)
+ for (int i = 0; i < Block8x8F.Size; i++)
{
result[Unzig[i]] = qt[i];
}
@@ -87,4 +95,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
return result;
}
}
-}
\ No newline at end of file
+}
diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs
index 442fcb3d1..56496fad1 100644
--- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs
+++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs
@@ -18,6 +18,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
// BUG: The following image has a high difference compared to the expected output:
// TestImages.Jpeg.Baseline.Jpeg420Small,
+ // BUG: While we can decode this image we do not return the same output as libjpeg
+ // based decoders and are inserting a number of lines equal to the corrupted lines
+ // below said lines.
+ // TestImages.Jpeg.Issues.Fuzz.AccessViolationException922,
+
TestImages.Jpeg.Baseline.Jpeg444,
TestImages.Jpeg.Baseline.Bad.BadEOF,
TestImages.Jpeg.Baseline.MultiScanBaselineCMYK,
@@ -106,4 +111,4 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
[TestImages.Jpeg.Progressive.Bad.ExifUndefType] = 0.011f / 100,
};
}
-}
\ No newline at end of file
+}
diff --git a/tests/ImageSharp.Tests/Formats/Jpg/ZigZagTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/ZigZagTests.cs
new file mode 100644
index 000000000..e59584991
--- /dev/null
+++ b/tests/ImageSharp.Tests/Formats/Jpg/ZigZagTests.cs
@@ -0,0 +1,44 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+using SixLabors.ImageSharp.Formats.Jpeg.Components;
+using Xunit;
+
+namespace SixLabors.ImageSharp.Tests.Formats.Jpg
+{
+ public class ZigZagTests
+ {
+ [Fact]
+ public void ZigZagCanHandleAllPossibleCoefficients()
+ {
+ // Mimic the behaviour of the huffman scan decoder using all possible byte values
+ short[] block = new short[64];
+ var zigzag = ZigZag.CreateUnzigTable();
+
+ for (int h = 0; h < 255; h++)
+ {
+ for (int i = 1; i < 64; i++)
+ {
+ int s = h;
+ int r = s >> 4;
+ s &= 15;
+
+ if (s != 0)
+ {
+ i += r;
+ block[zigzag[i++]] = (short)s;
+ }
+ else
+ {
+ if (r == 0)
+ {
+ break;
+ }
+
+ i += 16;
+ }
+ }
+ }
+ }
+ }
+}
diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs
index 62b7ae2ec..d8e8719ba 100644
--- a/tests/ImageSharp.Tests/TestImages.cs
+++ b/tests/ImageSharp.Tests/TestImages.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
// Licensed under the Apache License, Version 2.0.
using System.Linq;
@@ -202,6 +202,7 @@ namespace SixLabors.ImageSharp.Tests
public const string ArgumentException826C = "Jpg/issues/fuzz/Issue826-ArgumentException-C.jpg";
public const string AccessViolationException827 = "Jpg/issues/fuzz/Issue827-AccessViolationException.jpg";
public const string ExecutionEngineException839 = "Jpg/issues/fuzz/Issue839-ExecutionEngineException.jpg";
+ public const string AccessViolationException922 = "Jpg/issues/fuzz/Issue922-AccessViolationException.jpg";
}
}
diff --git a/tests/Images/Input/Jpg/issues/fuzz/Issue922-AccessViolationException.jpg b/tests/Images/Input/Jpg/issues/fuzz/Issue922-AccessViolationException.jpg
new file mode 100644
index 000000000..69f190629
Binary files /dev/null and b/tests/Images/Input/Jpg/issues/fuzz/Issue922-AccessViolationException.jpg differ