Skip to content

Bound setMediaStream search to the trak atom size#9286

Merged
kevinbackhouse merged 7 commits into
Exiv2:mainfrom
MarkLee131:fix/quicktime-setmediastream-bound
May 29, 2026
Merged

Bound setMediaStream search to the trak atom size#9286
kevinbackhouse merged 7 commits into
Exiv2:mainfrom
MarkLee131:fix/quicktime-setmediastream-bound

Conversation

@MarkLee131

Copy link
Copy Markdown
Contributor

Fix #9285:

setMediaStream() scans for "hdlr" in a while(!eof) loop with no size bound. When a file has no "hdlr" atom in the expected position, the loop reads through the entire rest of the file.

Pass the trak atom size and stop the search at that boundary.

setMediaStream() scans for "hdlr" in a while(!eof) loop with no
size bound. When a file has no "hdlr" atom in the expected
position, the loop reads through the entire rest of the file.

Pass the trak atom size and stop the search at that boundary.
Copilot AI review requested due to automatic review settings March 29, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses Exiv2 issue #9285 by bounding QuickTimeVideo::setMediaStream()’s search for the hdlr atom to the enclosing trak atom’s data size, preventing an unbounded scan across the rest of the file when hdlr is missing.

Changes:

  • Pass the trak atom data size into setMediaStream() from tagDecoder.
  • Add an explicit end position (search_end) and stop scanning once the trak boundary is reached.
  • Update the public header declaration to match the new setMediaStream(size_t) signature.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/quicktimevideo.cpp Bounds the hdlr scan within the trak payload and updates the call site to pass the size.
include/exiv2/quicktimevideo.hpp Updates setMediaStream declaration to accept the size parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/quicktimevideo.cpp
Comment on lines +1129 to +1133
void QuickTimeVideo::setMediaStream(size_t atom_size) {
size_t current_position = io_->tell();
size_t search_end = current_position + atom_size;
if (search_end > io_->size())
search_end = io_->size();

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new atom_size parameter is actually the box payload size (the size passed from tagDecoder is boxSize - headerSize). Consider renaming this parameter (and related locals like search_end) to reflect that it’s a payload/data-size bound, or update call sites to pass the full box size, to avoid future misuse/off-by-header confusion.

Copilot uses AI. Check for mistakes.
Comment thread src/quicktimevideo.cpp Outdated
Comment on lines +1136 to +1140
while (!io_->eof() && io_->tell() + 4 <= search_end) {
io_->readOrThrow(buf.data(), 4);
if (equalsQTimeTag(buf, "hdlr")) {
if (io_->tell() + 12 > search_end)
break;

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bounds checks use io_->tell() + N arithmetic (+4 in the loop condition and +12 when handling hdlr). Using addition on size_t can overflow; it would be safer to express these as io_->tell() <= search_end - N (after checking search_end >= N) or by computing remaining = search_end - io_->tell() as done elsewhere in this file (e.g., io_->size() - io_->tell()).

Suggested change
while (!io_->eof() && io_->tell() + 4 <= search_end) {
io_->readOrThrow(buf.data(), 4);
if (equalsQTimeTag(buf, "hdlr")) {
if (io_->tell() + 12 > search_end)
break;
while (!io_->eof()) {
// Ensure there are at least 4 bytes remaining before reading
size_t pos = io_->tell();
if (pos >= search_end)
break;
size_t remaining = search_end - pos;
if (remaining < 4)
break;
io_->readOrThrow(buf.data(), 4);
if (equalsQTimeTag(buf, "hdlr")) {
// After reading "hdlr", ensure there are at least 12 bytes remaining
pos = io_->tell();
if (pos >= search_end)
break;
remaining = search_end - pos;
if (remaining < 12)
break;

Copilot uses AI. Check for mistakes.
Comment thread src/quicktimevideo.cpp
Comment on lines +1129 to 1137
void QuickTimeVideo::setMediaStream(size_t atom_size) {
size_t current_position = io_->tell();
size_t search_end = current_position + atom_size;
if (search_end > io_->size())
search_end = io_->size();
DataBuf buf(4 + 1);

while (!io_->eof()) {
while (!io_->eof() && io_->tell() + 4 <= search_end) {
io_->readOrThrow(buf.data(), 4);

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setMediaStream now relies on the trak size bound to avoid scanning the rest of the file when hdlr is missing. There are existing QuickTime regression tests for long-running parsers under tests/bugfixes/github/; please add a new regression test (with a small PoC .mov/.mp4 fixture) that exercises a trak without an hdlr atom and ensures the command completes quickly.

Copilot uses AI. Check for mistakes.
Comment thread include/exiv2/quicktimevideo.hpp
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread src/quicktimevideo.cpp Outdated
Comment thread src/quicktimevideo.cpp Outdated
Comment thread src/quicktimevideo.cpp Outdated
kevinbackhouse and others added 5 commits May 23, 2026 22:36
Co-authored-by: Kevin Backhouse <kevinbackhouse@github.com>
Co-authored-by: Kevin Backhouse <kevinbackhouse@github.com>
Co-authored-by: Kevin Backhouse <kevinbackhouse@github.com>
@kevinbackhouse kevinbackhouse merged commit 9a2cff6 into Exiv2:main May 29, 2026
102 of 106 checks passed
@kevinbackhouse

Copy link
Copy Markdown
Collaborator

@mergify backport 0.28.x

@mergify

mergify Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

backport 0.28.x

✅ Backports have been created

Details

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.

QuickTimeVideo::setMediaStream() scans entire file when "hdlr" atom is missing

3 participants