Harden binary subtitle parsers against malformed files (crashes, OOM, infinite loops)#12581
Open
ivandrofly wants to merge 13 commits into
Open
Harden binary subtitle parsers against malformed files (crashes, OOM, infinite loops)#12581ivandrofly wants to merge 13 commits into
ivandrofly wants to merge 13 commits into
Conversation
A malformed ODS (e.g. declaring a 1x1 object but encoding long RLE runs) made the running pixel offset exceed the destination span, throwing IndexOutOfRangeException that escaped the decode try/catch. Clamp FillPixels/PutPixelFast writes to the span bounds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A truncated control sequence could end right after the SetColor (3) or SetContrast (4) command byte, so reading _data[commandIndex+1/+2] threw IndexOutOfRangeException. Guard both with the same length check the SetDisplayArea (5) case already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Truncated SEI payloads let ParseCcDataFromSei run past the buffer: an all-0xFF payloadType run, the 8-byte CC-data header probe, and the cc-triple loop all indexed without bounds checks, throwing IndexOutOfRangeException. Bound the payloadType loop and header read and clamp the triple loop to the buffer length. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When a NAL declares nalSize 0 or 1 it still passes the nalSize<10000 check, but endPos-startPos (= nalSize-2) underflows the ulong buffer length into a near-4GB allocation. Bail out when endPos <= startPos. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
stco, co64 and stts trusted file-declared entry counts and read past the box buffer, and stts additionally expanded a per-entry run length (sampleCount up to 0xFFFFFFFF) into an unbounded list. Add per-iteration bounds checks mirroring stsz/ctts/stsc and cap the stts expansion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A backspace at column 0 indexed Chars[-1] and a 33rd printable char indexed Chars[32], throwing IndexOutOfRangeException on malformed caption data. Clamp the cursor position to the valid column range in MoveCursor and InsertChar and skip out-of-range pen-state writes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A roll-up count larger than the current row (e.g. from malformed CEA-608 data) made RemoveAt use a negative index, throwing ArgumentOutOfRangeException. Bail out when the computed index is outside the rows list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ClearWindows/DisplayWindows/HideWindows/ToggleWindows/DeleteWindows and Delay (0x88-0x8D) read the argument byte at bytes[i+1] without checking the buffer length, so a command byte at the end of a truncated service block threw IndexOutOfRangeException. Add the same length guard the SetPen*/DefineWindow commands already use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetDvbSup indexed PageCompositions[0] before the Count>0 check on the next line, so a segment with no page composition threw IndexOutOfRangeException. Move the access inside the existing guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PES constructor read the fixed header bytes (buffer[index+6..8]) and the private-stream id at buffer[index+9+HeaderDataLength] without a length check, so a pack with a zero-length PES packet threw IndexOutOfRangeException. Bail out when the buffer is too short and guard the private-stream id read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A size-0 box (extends to EOF) set Position to fs.Length-8 instead of fs.Length because the remaining-bytes size did not include the header the Position formula subtracts, so the box-scan loops re-read the last 8 bytes forever. Add the header offset back and clamp Position so a malformed size can never point before the current position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When a CMAF file provides no positive time scale in either the track Mdhd or the Mvhd box, timeScale stayed 0 and the per-sample time conversion divided by zero, throwing DivideByZeroException. Bail out before the conversion loop when no usable time scale is present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A malformed PAC indent assigned directly to CcRow.Position (CaptionScreen sets row.Position = pacData.Indent.Value) was never range-checked, so SetPenStyles and the ClearFromPos loops could index the 32-entry Chars array out of bounds. Clamp the value in the Position setter so every reader stays within the valid column range. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds missing bounds/sanity guards across the binary parsers so malformed or truncated files degrade gracefully instead of crashing the import or hanging the app. Thirteen fixes, one commit each:
width*heightpixel span — a malformed ODS with long runs wrote past the buffer, and the exception escaped the per-segment try/catch via the post-parse merge path, aborting the file open.SetColor/SetContrast(commands 0x03/0x04) read two argument bytes with no bounds check, unlike the guarded sibling commands.MP4Parserconstructor with no try/catch);GetSeiDataalso underflowedendPos - startPosin ulong arithmetic for NAL sizes 0/1, attempting a huge allocation.CcRow.Positionnow clamps to the 32-column range in its setter (a malformed PAC indent could indexChars[-1]/Chars[32]),MoveCursor/InsertCharclamp locally, andRollUpguards a possibly-negativeRemoveAtindex.SetPen*/DefineWindowcommands.stco/co64/sttsloops bound file-declared entry counts against the actual buffer (mirroringstsz/ctts), and thesttsrun-length expansion is capped so asampleCountof 0xFFFFFFFF can no longer allocate toward 4 billion entries;Boxsize-0 ("extends to EOF") Position math is corrected and forward progress enforced so a malformed size can't loop forever; the CMAF parser bails on a zero time scale instead of dividing by zero.PageCompositions[0]before its ownCount > 0check; the PES constructor now validates the buffer is long enough for the fixed header and the private-stream-1 id byte (a declared packet length of 0 produced a too-short buffer).Build succeeds; all 499 libse tests pass.
🤖 Generated with Claude Code