Skip to content

Commit a54a19f

Browse files
Merge pull request #1924 from br3aker/dp/sof-exception-fix
Jpeg ArgumentException fix
2 parents 5fa5e50 + 22ed6da commit a54a19f

File tree

6 files changed

+98
-73
lines changed

6 files changed

+98
-73
lines changed

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ private JpegColorSpace DeduceJpegColorSpace(byte componentCount)
472472
: JpegColorSpace.Cmyk;
473473
}
474474

475-
JpegThrowHelper.ThrowInvalidImageContentException($"Unsupported color mode. Supported component counts 1, 3, and 4; found {componentCount}");
475+
JpegThrowHelper.ThrowNotSupportedComponentCount(componentCount);
476476
return default;
477477
}
478478

@@ -998,6 +998,14 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining,
998998
// 1 byte: Number of components
999999
byte componentCount = this.temp[5];
10001000

1001+
// Validate: componentCount more than 4 can lead to a buffer overflow during stream
1002+
// reading so we must limit it to 4
1003+
// We do not support jpeg images with more than 4 components anyway
1004+
if (componentCount > 4)
1005+
{
1006+
JpegThrowHelper.ThrowNotSupportedComponentCount(componentCount);
1007+
}
1008+
10011009
this.Frame = new JpegFrame(frameMarker, precision, frameWidth, frameHeight, componentCount);
10021010

10031011
remaining -= length;

src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,8 @@ internal static class JpegThrowHelper
4545

4646
[MethodImpl(InliningOptions.ColdPath)]
4747
public static void ThrowDimensionsTooLarge(int width, int height) => throw new ImageFormatException($"Image is too large to encode at {width}x{height} for JPEG format.");
48+
49+
[MethodImpl(InliningOptions.ColdPath)]
50+
public static void ThrowNotSupportedComponentCount(int componentCount) => throw new NotSupportedException($"Images with {componentCount} components are not supported.");
4851
}
4952
}

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs

Lines changed: 81 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -10,58 +10,72 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
1010
public partial class JpegDecoderTests
1111
{
1212
public static string[] BaselineTestJpegs =
13-
{
14-
TestImages.Jpeg.Baseline.Calliphora,
15-
TestImages.Jpeg.Baseline.Cmyk,
16-
TestImages.Jpeg.Baseline.Ycck,
17-
TestImages.Jpeg.Baseline.Jpeg400,
18-
TestImages.Jpeg.Baseline.Turtle420,
19-
TestImages.Jpeg.Baseline.Testorig420,
20-
TestImages.Jpeg.Baseline.Jpeg420Small,
21-
TestImages.Jpeg.Issues.Fuzz.AccessViolationException922,
22-
TestImages.Jpeg.Baseline.Jpeg444,
23-
TestImages.Jpeg.Baseline.Jpeg422,
24-
TestImages.Jpeg.Baseline.Bad.BadEOF,
25-
TestImages.Jpeg.Baseline.MultiScanBaselineCMYK,
26-
TestImages.Jpeg.Baseline.YcckSubsample1222,
27-
TestImages.Jpeg.Baseline.Bad.BadRST,
28-
TestImages.Jpeg.Issues.MultiHuffmanBaseline394,
29-
TestImages.Jpeg.Issues.ExifDecodeOutOfRange694,
30-
TestImages.Jpeg.Issues.InvalidEOI695,
31-
TestImages.Jpeg.Issues.ExifResizeOutOfRange696,
32-
TestImages.Jpeg.Issues.InvalidAPP0721,
33-
TestImages.Jpeg.Issues.ExifGetString750Load,
34-
TestImages.Jpeg.Issues.ExifGetString750Transform,
35-
TestImages.Jpeg.Issues.BadSubSampling1076,
13+
{
14+
TestImages.Jpeg.Baseline.Calliphora,
15+
TestImages.Jpeg.Baseline.Cmyk,
16+
TestImages.Jpeg.Baseline.Ycck,
17+
TestImages.Jpeg.Baseline.Jpeg400,
18+
TestImages.Jpeg.Baseline.Turtle420,
19+
TestImages.Jpeg.Baseline.Testorig420,
20+
TestImages.Jpeg.Baseline.Jpeg420Small,
21+
TestImages.Jpeg.Issues.Fuzz.AccessViolationException922,
22+
TestImages.Jpeg.Baseline.Jpeg444,
23+
TestImages.Jpeg.Baseline.Jpeg422,
24+
TestImages.Jpeg.Baseline.Bad.BadEOF,
25+
TestImages.Jpeg.Baseline.MultiScanBaselineCMYK,
26+
TestImages.Jpeg.Baseline.YcckSubsample1222,
27+
TestImages.Jpeg.Baseline.Bad.BadRST,
28+
TestImages.Jpeg.Issues.MultiHuffmanBaseline394,
29+
TestImages.Jpeg.Issues.ExifDecodeOutOfRange694,
30+
TestImages.Jpeg.Issues.InvalidEOI695,
31+
TestImages.Jpeg.Issues.ExifResizeOutOfRange696,
32+
TestImages.Jpeg.Issues.InvalidAPP0721,
33+
TestImages.Jpeg.Issues.ExifGetString750Load,
34+
TestImages.Jpeg.Issues.ExifGetString750Transform,
35+
TestImages.Jpeg.Issues.BadSubSampling1076,
3636

37-
// LibJpeg can open this despite the invalid density units.
38-
TestImages.Jpeg.Issues.Fuzz.ArgumentOutOfRangeException825B,
37+
// LibJpeg can open this despite the invalid density units.
38+
TestImages.Jpeg.Issues.Fuzz.ArgumentOutOfRangeException825B,
3939

40-
// LibJpeg can open this despite incorrect colorspace metadata.
41-
TestImages.Jpeg.Issues.IncorrectColorspace855,
40+
// LibJpeg can open this despite incorrect colorspace metadata.
41+
TestImages.Jpeg.Issues.IncorrectColorspace855,
4242

43-
// High depth images
44-
TestImages.Jpeg.Baseline.Testorig12bit,
45-
};
43+
// High depth images
44+
TestImages.Jpeg.Baseline.Testorig12bit,
45+
};
4646

4747
public static string[] ProgressiveTestJpegs =
48-
{
49-
TestImages.Jpeg.Progressive.Fb,
50-
TestImages.Jpeg.Progressive.Progress,
51-
TestImages.Jpeg.Progressive.Festzug,
52-
TestImages.Jpeg.Progressive.Bad.BadEOF,
53-
TestImages.Jpeg.Issues.BadCoeffsProgressive178,
54-
TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159,
55-
TestImages.Jpeg.Issues.MissingFF00ProgressiveBedroom159,
56-
TestImages.Jpeg.Issues.BadZigZagProgressive385,
57-
TestImages.Jpeg.Progressive.Bad.ExifUndefType,
58-
TestImages.Jpeg.Issues.NoEoiProgressive517,
59-
TestImages.Jpeg.Issues.BadRstProgressive518,
60-
TestImages.Jpeg.Issues.DhtHasWrongLength624,
61-
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723A,
62-
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723B,
63-
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723C
64-
};
48+
{
49+
TestImages.Jpeg.Progressive.Fb,
50+
TestImages.Jpeg.Progressive.Progress,
51+
TestImages.Jpeg.Progressive.Festzug,
52+
TestImages.Jpeg.Progressive.Bad.BadEOF,
53+
TestImages.Jpeg.Issues.BadCoeffsProgressive178,
54+
TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159,
55+
TestImages.Jpeg.Issues.MissingFF00ProgressiveBedroom159,
56+
TestImages.Jpeg.Issues.BadZigZagProgressive385,
57+
TestImages.Jpeg.Progressive.Bad.ExifUndefType,
58+
TestImages.Jpeg.Issues.NoEoiProgressive517,
59+
TestImages.Jpeg.Issues.BadRstProgressive518,
60+
TestImages.Jpeg.Issues.DhtHasWrongLength624,
61+
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723A,
62+
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723B,
63+
TestImages.Jpeg.Issues.OrderedInterleavedProgressive723C
64+
};
65+
66+
public static string[] UnsupportedTestJpegs =
67+
{
68+
// Invalid componentCount value (2 or > 4)
69+
TestImages.Jpeg.Issues.Fuzz.NullReferenceException823,
70+
TestImages.Jpeg.Issues.MalformedUnsupportedComponentCount,
71+
72+
// Arithmetic coding
73+
TestImages.Jpeg.Baseline.ArithmeticCoding,
74+
TestImages.Jpeg.Baseline.ArithmeticCodingProgressive,
75+
76+
// Lossless jpeg
77+
TestImages.Jpeg.Baseline.Lossless
78+
};
6579

6680
public static string[] UnrecoverableTestJpegs =
6781
{
@@ -70,7 +84,6 @@ public partial class JpegDecoderTests
7084
TestImages.Jpeg.Issues.Fuzz.AccessViolationException798,
7185
TestImages.Jpeg.Issues.Fuzz.DivideByZeroException821,
7286
TestImages.Jpeg.Issues.Fuzz.DivideByZeroException822,
73-
TestImages.Jpeg.Issues.Fuzz.NullReferenceException823,
7487
TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824A,
7588
TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824B,
7689
TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824D,
@@ -91,28 +104,27 @@ public partial class JpegDecoderTests
91104
TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824C,
92105
};
93106

94-
private static readonly Dictionary<string, float> CustomToleranceValues =
95-
new Dictionary<string, float>
96-
{
97-
// Baseline:
98-
[TestImages.Jpeg.Baseline.Calliphora] = 0.00002f / 100,
99-
[TestImages.Jpeg.Baseline.Bad.BadEOF] = 0.38f / 100,
100-
[TestImages.Jpeg.Baseline.Bad.BadRST] = 0.0589f / 100,
107+
private static readonly Dictionary<string, float> CustomToleranceValues = new()
108+
{
109+
// Baseline:
110+
[TestImages.Jpeg.Baseline.Calliphora] = 0.00002f / 100,
111+
[TestImages.Jpeg.Baseline.Bad.BadEOF] = 0.38f / 100,
112+
[TestImages.Jpeg.Baseline.Bad.BadRST] = 0.0589f / 100,
101113

102-
[TestImages.Jpeg.Baseline.Jpeg422] = 0.0013f / 100,
103-
[TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100,
104-
[TestImages.Jpeg.Baseline.Jpeg420Small] = 0.287f / 100,
105-
[TestImages.Jpeg.Baseline.Turtle420] = 1.0f / 100,
114+
[TestImages.Jpeg.Baseline.Jpeg422] = 0.0013f / 100,
115+
[TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100,
116+
[TestImages.Jpeg.Baseline.Jpeg420Small] = 0.287f / 100,
117+
[TestImages.Jpeg.Baseline.Turtle420] = 1.0f / 100,
106118

107-
// Progressive:
108-
[TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159] = 0.34f / 100,
109-
[TestImages.Jpeg.Issues.BadCoeffsProgressive178] = 0.38f / 100,
110-
[TestImages.Jpeg.Progressive.Bad.BadEOF] = 0.3f / 100,
111-
[TestImages.Jpeg.Progressive.Festzug] = 0.02f / 100,
112-
[TestImages.Jpeg.Progressive.Fb] = 0.16f / 100,
113-
[TestImages.Jpeg.Progressive.Progress] = 0.31f / 100,
114-
[TestImages.Jpeg.Issues.BadZigZagProgressive385] = 0.23f / 100,
115-
[TestImages.Jpeg.Progressive.Bad.ExifUndefType] = 0.011f / 100,
116-
};
119+
// Progressive:
120+
[TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159] = 0.34f / 100,
121+
[TestImages.Jpeg.Issues.BadCoeffsProgressive178] = 0.38f / 100,
122+
[TestImages.Jpeg.Progressive.Bad.BadEOF] = 0.3f / 100,
123+
[TestImages.Jpeg.Progressive.Festzug] = 0.02f / 100,
124+
[TestImages.Jpeg.Progressive.Fb] = 0.16f / 100,
125+
[TestImages.Jpeg.Progressive.Progress] = 0.31f / 100,
126+
[TestImages.Jpeg.Issues.BadZigZagProgressive385] = 0.23f / 100,
127+
[TestImages.Jpeg.Progressive.Bad.ExifUndefType] = 0.011f / 100,
128+
};
117129
}
118130
}

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,7 @@ public async Task Identify_IsCancellable()
190190
}
191191

192192
[Theory]
193-
[WithFile(TestImages.Jpeg.Baseline.ArithmeticCoding, PixelTypes.Rgba32)]
194-
[WithFile(TestImages.Jpeg.Baseline.ArithmeticCodingProgressive, PixelTypes.Rgba32)]
195-
[WithFile(TestImages.Jpeg.Baseline.Lossless, PixelTypes.Rgba32)]
193+
[WithFileCollection(nameof(UnsupportedTestJpegs), PixelTypes.Rgba32)]
196194
public void ThrowsNotSupported_WithUnsupportedJpegs<TPixel>(TestImageProvider<TPixel> provider)
197195
where TPixel : unmanaged, IPixel<TPixel>
198196
{

tests/ImageSharp.Tests/TestImages.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ public static class Issues
256256
public const string BadSubSampling1076 = "Jpg/issues/issue-1076-invalid-subsampling.jpg";
257257
public const string IdentifyMultiFrame1211 = "Jpg/issues/issue-1221-identify-multi-frame.jpg";
258258
public const string WrongColorSpace = "Jpg/issues/Issue1732-WrongColorSpace.jpg";
259+
public const string MalformedUnsupportedComponentCount = "Jpg/issues/issue-1900-malformed-unsupported-255-components.jpg";
259260

260261
public static class Fuzz
261262
{
Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)