Skip to content

Skip stray TEM markers and reserved markers before restart markers when decoding JPEGs - #3225

Open
jafin wants to merge 4 commits into
SixLabors:mainfrom
jafin:jf/jpeg-marker-resync
Open

jafin wants to merge 4 commits into
SixLabors:mainfrom
jafin:jf/jpeg-marker-resync

Conversation

@jafin

@jafin jafin commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

JPEG header parsing treated the standalone TEM marker as a segment with a length field. It now skips TEM without consuming bytes from the next segment. This applies to both identification and decoding under every integrity policy. ITU T.81 Table B.1 defines TEM as a standalone marker.

At restart boundaries, TEM and reserved marker codes could end a scan early. The shared JpegBitReader now scans past those codes only when SegmentIntegrityHandling.IgnoreImageData permits recovery. Strict and IgnoreAncillary reject them. Real segment markers remain available to the segment parser. Both Huffman and arithmetic scans use this path. Arithmetic restart handling also classifies the detected marker before resetting the reader, so it does not lose the next segment marker.

The cases are in the existing JpegDecoderTests.cs, beside the extraneous-marker regression. The added standalone test file is removed. Tests load stored inputs; they do not encode or modify images at runtime. Local libjpeg-turbo 3.1.91 generated the clean inputs. Its decoder produces identical pixels for each clean/corrupted pair, with warnings for the corrupted inputs. ImageSharp checks exact pixel equality for recovery and checks rejection under the other policies. Header tests also check that an invalid required DQT length remains fatal in every mode.

The arithmetic inputs declare their conditioning tables explicitly. A separate existing defect rejects progressive arithmetic input when it relies on the standard AC conditioning defaults. This PR does not change that behavior.

Validation: Release JPEG and TIFF tests pass on .NET 10 and .NET 11: 1,571 passed, one existing debug-only skip, zero failed on each target. No tolerances, reference images, or BufferedReadStream code changed.

V4 backport: #3258.

Fixes #3237

…en decoding jpegs

A TEM marker (FF 01) between header segments was read as a marker with a
length field, so the next segment header was consumed as its length and
decoding failed. TEM stands alone like RSTn, so skip it the same way.

Some producers write extraneous bytes before restart markers. Those that
form a TEM or reserved marker (FF 01 to FF BF) were taken as the end of the
scan, which corrupted the rest of the image or made the marker parser fail.
Skip them while looking for the marker that ends a restart interval and
resync at the next marker, as libjpeg does.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JPEG decoder fails or corrupts the image when a stray TEM marker or reserved marker bytes appear in the stream

2 participants