Skip to content

Commit 8f3658d

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Jpeg Fuzz Fixes (#836)
* Nomalize jpeg exceptions. Fix #821 * Fix #822 * Fix #823 * Check for correct QT index. Touch #824 * Check DHT props. Touch #824 * Limit sampling factors to 1 & 2. Touch #824 * Add already fixed image 4. Touch #824 * Check for excessive code lengths. Touch #824 * Add already fixed image 6. Touch #824 * Lint progressive scan details. Touch #824 * Add already fixed image 8. Fix #824 * Remove duplicate per-block checks * Add already fixed image 1. Touch #825 * Don't throw on bad JFIF density units. * Add already fixed image 3. Touch #825 * Add already fixed image 4. Fix #825 * Check SOFn marker length. Touch #826 * Add already fixed image 2. Touch #826 * Add already fixed image 3. Fix #826 * Add fixed already fixed image. Fix #827 * Revert unneeded bounds check introduced in #804
1 parent 5a9b84e commit 8f3658d

33 files changed

+246
-85
lines changed

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,8 @@ internal unsafe struct HuffmanTable
5050
/// <param name="values">The huffman values</param>
5151
public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLengths, ReadOnlySpan<byte> values)
5252
{
53-
// We do some bounds checks in the code here to protect against AccessViolationExceptions
54-
const int HuffCodeLength = 257;
55-
const int MaxSizeLength = HuffCodeLength - 1;
56-
using (IMemoryOwner<short> huffcode = memoryAllocator.Allocate<short>(HuffCodeLength))
53+
const int Length = 257;
54+
using (IMemoryOwner<short> huffcode = memoryAllocator.Allocate<short>(Length))
5755
{
5856
ref short huffcodeRef = ref MemoryMarshal.GetReference(huffcode.GetSpan());
5957
ref byte codeLengthsRef = ref MemoryMarshal.GetReference(codeLengths);
@@ -65,7 +63,7 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng
6563
for (short i = 1; i < 17; i++)
6664
{
6765
byte length = Unsafe.Add(ref codeLengthsRef, i);
68-
for (short j = 0; j < length && x < MaxSizeLength; j++)
66+
for (short j = 0; j < length; j++)
6967
{
7068
Unsafe.Add(ref sizesRef, x++) = i;
7169
}
@@ -86,7 +84,7 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng
8684
Unsafe.Add(ref valOffsetRef, k) = (int)(si - code);
8785
if (Unsafe.Add(ref sizesRef, si) == k)
8886
{
89-
while (Unsafe.Add(ref sizesRef, si) == k && si < HuffCodeLength)
87+
while (Unsafe.Add(ref sizesRef, si) == k)
9088
{
9189
Unsafe.Add(ref huffcodeRef, si++) = (short)code++;
9290
}
@@ -103,19 +101,19 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng
103101
// Generate non-spec lookup tables to speed up decoding.
104102
const int FastBits = ScanDecoder.FastBits;
105103
ref byte lookaheadRef = ref this.Lookahead[0];
106-
const uint MaxFastLength = 1 << FastBits;
107-
Unsafe.InitBlockUnaligned(ref lookaheadRef, 0xFF, MaxFastLength); // Flag for non-accelerated
104+
Unsafe.InitBlockUnaligned(ref lookaheadRef, 0xFF, 1 << FastBits); // Flag for non-accelerated
108105

109106
for (int i = 0; i < si; i++)
110107
{
111108
int size = Unsafe.Add(ref sizesRef, i);
112109
if (size <= FastBits)
113110
{
114-
int huffCode = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size);
115-
int max = 1 << (FastBits - size);
116-
for (int left = 0; left < max; left++)
111+
int fastOffset = FastBits - size;
112+
int fastCode = Unsafe.Add(ref huffcodeRef, i) << fastOffset;
113+
int fastMax = 1 << fastOffset;
114+
for (int left = 0; left < fastMax; left++)
117115
{
118-
Unsafe.Add(ref lookaheadRef, huffCode + left) = (byte)i;
116+
Unsafe.Add(ref lookaheadRef, fastCode + left) = (byte)i;
119117
}
120118
}
121119
}

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,20 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
2727
/// <param name="yDensity">The vertical pixel density</param>
2828
private JFifMarker(byte majorVersion, byte minorVersion, byte densityUnits, short xDensity, short yDensity)
2929
{
30-
Guard.MustBeGreaterThan(xDensity, 0, nameof(xDensity));
31-
Guard.MustBeGreaterThan(yDensity, 0, nameof(yDensity));
32-
Guard.MustBeBetweenOrEqualTo(densityUnits, 0, 2, nameof(densityUnits));
30+
if (xDensity <= 0)
31+
{
32+
JpegThrowHelper.ThrowImageFormatException($"X-Density {xDensity} must be greater than 0.");
33+
}
34+
35+
if (yDensity <= 0)
36+
{
37+
JpegThrowHelper.ThrowImageFormatException($"Y-Density {yDensity} must be greater than 0.");
38+
}
3339

3440
this.MajorVersion = majorVersion;
3541
this.MinorVersion = minorVersion;
42+
43+
// LibJpeg and co will simply cast and not try to enforce a range.
3644
this.DensityUnits = (PixelResolutionUnit)densityUnits;
3745
this.XDensity = xDensity;
3846
this.YDensity = yDensity;
@@ -104,10 +112,7 @@ public bool Equals(JFifMarker other)
104112
}
105113

106114
/// <inheritdoc/>
107-
public override bool Equals(object obj)
108-
{
109-
return obj is JFifMarker other && this.Equals(other);
110-
}
115+
public override bool Equals(object obj) => obj is JFifMarker other && this.Equals(other);
111116

112117
/// <inheritdoc/>
113118
public override int GetHashCode()

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,25 @@ public JpegComponent(MemoryAllocator memoryAllocator, JpegFrame frame, byte id,
2121
this.memoryAllocator = memoryAllocator;
2222
this.Frame = frame;
2323
this.Id = id;
24+
25+
// Valid sampling factors are 1..2
26+
if (horizontalFactor == 0
27+
|| verticalFactor == 0
28+
|| horizontalFactor > 2
29+
|| verticalFactor > 2)
30+
{
31+
JpegThrowHelper.ThrowBadSampling();
32+
}
33+
2434
this.HorizontalSamplingFactor = horizontalFactor;
2535
this.VerticalSamplingFactor = verticalFactor;
2636
this.SamplingFactors = new Size(this.HorizontalSamplingFactor, this.VerticalSamplingFactor);
37+
38+
if (quantizationTableIndex > 3)
39+
{
40+
JpegThrowHelper.ThrowBadQuantizationTable();
41+
}
42+
2743
this.QuantizationTableIndex = quantizationTableIndex;
2844
this.Index = index;
2945
}
@@ -110,6 +126,11 @@ public void Init()
110126
JpegComponent c0 = this.Frame.Components[0];
111127
this.SubSamplingDivisors = c0.SamplingFactors.DivideBy(this.SamplingFactors);
112128

129+
if (this.SubSamplingDivisors.Width == 0 || this.SubSamplingDivisors.Height == 0)
130+
{
131+
JpegThrowHelper.ThrowBadSampling();
132+
}
133+
113134
int totalNumberOfBlocks = blocksPerColumnForMcu * (blocksPerLineForMcu + 1);
114135
int width = this.WidthInBlocks + 1;
115136
int height = totalNumberOfBlocks / width;

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void Dispose()
8383
{
8484
for (int i = 0; i < this.Components.Length; i++)
8585
{
86-
this.Components[i].Dispose();
86+
this.Components[i]?.Dispose();
8787
}
8888

8989
this.Components = null;

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,60 @@ private void ParseBaselineDataNonInterleaved()
270270
}
271271
}
272272

273+
private void CheckProgressiveData()
274+
{
275+
// Validate successive scan parameters.
276+
// Logic has been adapted from libjpeg.
277+
// See Table B.3 – Scan header parameter size and values. itu-t81.pdf
278+
bool invalid = false;
279+
if (this.spectralStart == 0)
280+
{
281+
if (this.spectralEnd != 0)
282+
{
283+
invalid = true;
284+
}
285+
}
286+
else
287+
{
288+
// Need not check Ss/Se < 0 since they came from unsigned bytes.
289+
if (this.spectralEnd < this.spectralStart || this.spectralEnd > 63)
290+
{
291+
invalid = true;
292+
}
293+
294+
// AC scans may have only one component.
295+
if (this.componentsLength != 1)
296+
{
297+
invalid = true;
298+
}
299+
}
300+
301+
if (this.successiveHigh != 0)
302+
{
303+
// Successive approximation refinement scan: must have Al = Ah-1.
304+
if (this.successiveHigh - 1 != this.successiveLow)
305+
{
306+
invalid = true;
307+
}
308+
}
309+
310+
// TODO: How does this affect 12bit jpegs.
311+
// According to libjpeg the range covers 8bit only?
312+
if (this.successiveLow > 13)
313+
{
314+
invalid = true;
315+
}
316+
317+
if (invalid)
318+
{
319+
JpegThrowHelper.ThrowBadProgressiveScan(this.spectralStart, this.spectralEnd, this.successiveHigh, this.successiveLow);
320+
}
321+
}
322+
273323
private void ParseProgressiveData()
274324
{
325+
this.CheckProgressiveData();
326+
275327
if (this.componentsLength == 1)
276328
{
277329
this.ParseProgressiveDataNonInterleaved();
@@ -483,11 +535,6 @@ private void DecodeBlockProgressiveDC(
483535
ref Block8x8 block,
484536
ref HuffmanTable dcTable)
485537
{
486-
if (this.spectralEnd != 0)
487-
{
488-
JpegThrowHelper.ThrowImageFormatException("Can't merge DC and AC.");
489-
}
490-
491538
this.CheckBits();
492539

493540
ref short blockDataRef = ref Unsafe.As<Block8x8, short>(ref block);
@@ -518,11 +565,6 @@ private void DecodeBlockProgressiveAC(
518565
ref HuffmanTable acTable,
519566
ref short fastACRef)
520567
{
521-
if (this.spectralStart == 0)
522-
{
523-
JpegThrowHelper.ThrowImageFormatException("Can't merge DC and AC.");
524-
}
525-
526568
ref short blockDataRef = ref Unsafe.As<Block8x8, short>(ref block);
527569

528570
if (this.successiveHigh == 0)

0 commit comments

Comments
 (0)