diff --git a/Libraries/LibMedia/Containers/Matroska/Reader.cpp b/Libraries/LibMedia/Containers/Matroska/Reader.cpp index 95634d6d3b..430ab5dfe8 100644 --- a/Libraries/LibMedia/Containers/Matroska/Reader.cpp +++ b/Libraries/LibMedia/Containers/Matroska/Reader.cpp @@ -804,7 +804,10 @@ DecoderErrorOr> 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) { diff --git a/Tests/LibMedia/TestParseMatroska.cpp b/Tests/LibMedia/TestParseMatroska.cpp index 33bcdc8634..9cb3cd5302 100644 --- a/Tests/LibMedia/TestParseMatroska.cpp +++ b/Tests/LibMedia/TestParseMatroska.cpp @@ -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 { + 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 { + 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()); +} diff --git a/Tests/LibMedia/test-matroska-fixed-size-lacing-invalid.mkv b/Tests/LibMedia/test-matroska-fixed-size-lacing-invalid.mkv new file mode 100644 index 0000000000..77e6ec534a Binary files /dev/null and b/Tests/LibMedia/test-matroska-fixed-size-lacing-invalid.mkv differ diff --git a/Tests/LibMedia/test-matroska-fixed-size-lacing.mkv b/Tests/LibMedia/test-matroska-fixed-size-lacing.mkv new file mode 100644 index 0000000000..151e62205d Binary files /dev/null and b/Tests/LibMedia/test-matroska-fixed-size-lacing.mkv differ