Harden the movie decoder against malformed ANIM data - #565
Merged
bibendovsky merged 2 commits intoAug 4, 2026
Conversation
AdamCoulterOz
force-pushed
the
fix/movie-playback-hardening
branch
from
August 4, 2026 03:40
ec6a3ae to
c611639
Compare
The record size of an ANIM frame is read straight from the asset and then trusted twice over: AN_PAGE hands it to a sub-reader as the size of the frame payload, and every handler advances the reader by it once the frame is done. Neither use was checked. A record size larger than the bytes left in the file gives the sub-reader a length that reaches past the end of the buffer the file was read into, and the chunk loop then walks - and blits - heap memory beyond it. A negative one is worse: the reader skips backwards, so the ten bytes of frame header are re-read on the next iteration and the same frame plays forever with no way out, because neither the sound nor the pause handler polls for input. Check it once where it is read instead. The sound and pause handlers now also require their two byte payload to fit inside the record rather than merely inside the file, so a short record can no longer borrow the first bytes of the following frame header. All four shipped animations pass the new check unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An AN_PAGE chunk carries a sixteen bit offset and a sixteen bit length, both taken from the asset as is. The only check on them bounds the source read; nothing bounds the destination. VL_Plot does no clamping at all, so a chunk ending past pixel 63999 writes attacker chosen bytes past the end of both the UI buffer and the mask array - a chunk can reach pixel 131070, more than twice the screen. Reject such a chunk where the rest of the frame is validated. The blitter stays a plain blitter, and all four shipped animations stay well inside the screen, the furthest reaching pixel 52724. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AdamCoulterOz
force-pushed
the
fix/movie-playback-hardening
branch
from
August 4, 2026 04:08
c611639 to
5aef667
Compare
Owner
|
Thank you! |
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
2 participants
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.
From a correctness review of wip: the movie reader trusted two values it read out of the file.
🤖 Generated with Claude Code