LibMedia: Validate fixed-size Matroska frames

We were allowing Matroska blocks with fixed-size lacing to contain
frames with non-divisible sizes. This should not be possible, as it
inherently means that trailing bytes will be discarded.

We now have a valid and invalid testcase for fixed-size lacing to
ensure our handling remains correct.
This commit is contained in:
Zaggy1024 2026-01-28 12:38:54 -06:00 committed by Gregory Bertilson
parent bc67764606
commit ee95de40d6
4 changed files with 67 additions and 1 deletions

View file

@ -804,7 +804,10 @@ DecoderErrorOr<Vector<ByteBuffer>> SampleIterator::get_frames(Block block)
}
} else if (block.lacing() == Block::Lacing::FixedSize) {
auto frame_count = TRY(streamer.read_octet()) + 1;
auto individual_frame_size = block.data_size() / frame_count;
auto frames_data_size = block.data_size() - 1;
if ((frames_data_size % frame_count) != 0)
return DecoderError::corrupted("Block with fixed-size frames has non-divisible size"sv);
auto individual_frame_size = frames_data_size / frame_count;
for (int i = 0; i < frame_count; i++)
frames.append(TRY(streamer.read_raw_octets(individual_frame_size)));
} else if (block.lacing() == Block::Lacing::XIPH) {

View file

@ -76,3 +76,66 @@ TEST_CASE(block_group)
EXPECT_EQ(second_block.timestamp().to_milliseconds(), 33);
EXPECT(second_block.only_keyframes());
}
TEST_CASE(fixed_size_lacing)
{
auto file = MUST(Core::File::open("./test-matroska-fixed-size-lacing.mkv"sv, Core::File::OpenMode::Read));
auto stream = Media::IncrementallyPopulatedStream::create_from_buffer(MUST(file->read_until_eof()));
auto matroska_reader = MUST(Media::Matroska::Reader::from_stream(stream->create_cursor()));
u64 video_track = 0;
MUST(matroska_reader.for_each_track_of_type(Media::Matroska::TrackEntry::TrackType::Video, [&](Media::Matroska::TrackEntry const& track_entry) -> Media::DecoderErrorOr<IterationDecision> {
video_track = track_entry.track_number();
return IterationDecision::Break;
}));
EXPECT_EQ(video_track, 1u);
auto iterator = MUST(matroska_reader.create_sample_iterator(stream->create_cursor(), video_track));
// Block 1: 4 frames × 4 bytes
auto block1 = MUST(iterator.next_block());
EXPECT_EQ(block1.timestamp().to_milliseconds(), 0);
EXPECT(block1.only_keyframes());
EXPECT_EQ(block1.lacing(), Media::Matroska::Block::Lacing::FixedSize);
auto frames1 = MUST(iterator.get_frames(block1));
EXPECT_EQ(frames1.size(), 4u);
for (auto const& frame : frames1)
EXPECT_EQ(frame.size(), 4u);
// Block 2: 2 frames × 8 bytes
auto block2 = MUST(iterator.next_block());
EXPECT_EQ(block2.timestamp().to_milliseconds(), 33);
EXPECT_EQ(block2.lacing(), Media::Matroska::Block::Lacing::FixedSize);
auto frames2 = MUST(iterator.get_frames(block2));
EXPECT_EQ(frames2.size(), 2u);
for (auto const& frame : frames2)
EXPECT_EQ(frame.size(), 8u);
// Block 3: 3 frames × 1 byte
auto block3 = MUST(iterator.next_block());
EXPECT_EQ(block3.timestamp().to_milliseconds(), 66);
EXPECT_EQ(block3.lacing(), Media::Matroska::Block::Lacing::FixedSize);
auto frames3 = MUST(iterator.get_frames(block3));
EXPECT_EQ(frames3.size(), 3u);
for (auto const& frame : frames3)
EXPECT_EQ(frame.size(), 1u);
}
TEST_CASE(fixed_size_lacing_invalid)
{
auto file = MUST(Core::File::open("./test-matroska-fixed-size-lacing-invalid.mkv"sv, Core::File::OpenMode::Read));
auto stream = Media::IncrementallyPopulatedStream::create_from_buffer(MUST(file->read_until_eof()));
auto matroska_reader = MUST(Media::Matroska::Reader::from_stream(stream->create_cursor()));
u64 video_track = 0;
MUST(matroska_reader.for_each_track_of_type(Media::Matroska::TrackEntry::TrackType::Video, [&](Media::Matroska::TrackEntry const& track_entry) -> Media::DecoderErrorOr<IterationDecision> {
video_track = track_entry.track_number();
return IterationDecision::Break;
}));
EXPECT_EQ(video_track, 1u);
auto iterator = MUST(matroska_reader.create_sample_iterator(stream->create_cursor(), video_track));
auto block = MUST(iterator.next_block());
EXPECT_EQ(block.lacing(), Media::Matroska::Block::Lacing::FixedSize);
auto frames_or_error = iterator.get_frames(block);
EXPECT(frames_or_error.is_error());
}

Binary file not shown.