[vlc-commits] [Git][videolan/vlc][master] contrib: matroska: rework EBML lacing partial reading
Steve Lhomme (@robUx4)
gitlab at videolan.org
Fri May 1 10:36:08 UTC 2026
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
82fd4193 by Steve Lhomme at 2026-05-01T10:02:18+00:00
contrib: matroska: rework EBML lacing partial reading
We shouldn't read more than the size of EBML lacing size(s).
And we should especially not guesstimate the EBML lacing size will take.
It usually much less than the guesstimate.
- - - - -
4 changed files:
- − contrib/src/matroska/0001-KaxBlock-fix-leak-when-reading-EBML-lace-is-aborted.patch
- + contrib/src/matroska/0001-KaxBlock-rework-EBML-lacing-sizes-in-SCOPE_PARTIAL_D.patch
- + contrib/src/matroska/0002-KaxBlock-throw-when-the-EBML-length-difference-gives.patch
- contrib/src/matroska/rules.mak
Changes:
=====================================
contrib/src/matroska/0001-KaxBlock-fix-leak-when-reading-EBML-lace-is-aborted.patch deleted
=====================================
@@ -1,59 +0,0 @@
-From 6d2ad6dfb9d16a7747cc8395b022fc20eb91d3ec Mon Sep 17 00:00:00 2001
-From: Steve Lhomme <slhomme at matroska.org>
-Date: Fri, 24 Oct 2025 11:18:54 +0200
-Subject: [PATCH 1/2] KaxBlock: fix leak when reading EBML lace is aborted
-
----
- src/KaxBlock.cpp | 11 +++++++----
- 1 file changed, 7 insertions(+), 4 deletions(-)
-
-diff --git a/src/KaxBlock.cpp b/src/KaxBlock.cpp
-index 62b0947..6a61b73 100644
---- a/src/KaxBlock.cpp
-+++ b/src/KaxBlock.cpp
-@@ -44,6 +44,8 @@
- #include "matroska/KaxCluster.h"
- #include "matroska/KaxDefines.h"
-
-+#include <memory>
-+
- namespace libmatroska {
-
- DataBuffer * DataBuffer::Clone()
-@@ -582,7 +584,6 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
- if (Result != 5)
- throw SafeReadIOCallback::EndOfStreamX(0);
- binary *cursor = _TempHead;
-- binary *_tmpBuf;
- uint8 BlockHeadSize = 4;
-
- // update internal values
-@@ -656,8 +657,10 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
- SizeList[Index] = LastBufferSize;
- break;
- case LACING_EBML:
-+ {
- SizeRead = LastBufferSize;
-- cursor = _tmpBuf = new binary[FrameNum*4]; /// \warning assume the mean size will be coded in less than 4 bytes
-+ auto _tmpBuf = std::make_unique<binary>(FrameNum*4); /// \warning assume the mean size will be coded in less than 4 bytes
-+ cursor = _tmpBuf.get();
- Result += input.read(cursor, FrameNum*4);
- FrameSize = ReadCodedSizeValue(cursor, SizeRead, SizeUnknown);
- if (FrameSize > TotalLacedSize)
-@@ -677,11 +680,11 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
- LastBufferSize -= FrameSize + SizeRead;
- }
-
-- FirstFrameLocation += cursor - _tmpBuf;
-+ FirstFrameLocation += cursor - _tmpBuf.get();
-
- SizeList[Index] = LastBufferSize;
-- delete [] _tmpBuf;
- break;
-+ }
- case LACING_FIXED:
- for (Index=0; Index<=FrameNum; Index++) {
- // get the size of the frame
---
-2.52.0.windows.1
-
=====================================
contrib/src/matroska/0001-KaxBlock-rework-EBML-lacing-sizes-in-SCOPE_PARTIAL_D.patch
=====================================
@@ -0,0 +1,113 @@
+From 058046e606eb60fcc0ce0ef3801ed11cf7e0e997 Mon Sep 17 00:00:00 2001
+From: Steve Lhomme <robux4 at ycbcr.xyz>
+Date: Tue, 28 Apr 2026 10:08:39 +0200
+Subject: [PATCH 1/2] KaxBlock: rework EBML lacing sizes in SCOPE_PARTIAL_DATA
+ mode
+
+We only read the bytes that correspond to frame sizes, rather
+than a guesstimate of how much data we are going to need.
+
+That's what the SCOPE_PARTIAL_DATA mode is about. This is already the case
+for LACING_XIPH. And we don't need a buffer allocation.
+---
+ src/KaxBlock.cpp | 63 ++++++++++++++++++++++++++++++++++++++----------
+ 1 file changed, 50 insertions(+), 13 deletions(-)
+
+diff --git a/src/KaxBlock.cpp b/src/KaxBlock.cpp
+index fd37ad5..5fab622 100644
+--- a/src/KaxBlock.cpp
++++ b/src/KaxBlock.cpp
+@@ -582,7 +582,6 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
+ if (Result != 5)
+ throw SafeReadIOCallback::EndOfStreamX(0);
+ binary *cursor = _TempHead;
+- binary *_tmpBuf;
+ uint8 BlockHeadSize = 4;
+
+ // update internal values
+@@ -656,32 +655,70 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
+ SizeList[Index] = LastBufferSize;
+ break;
+ case LACING_EBML:
+- SizeRead = LastBufferSize;
+- cursor = _tmpBuf = new binary[FrameNum*4]; /// \warning assume the mean size will be coded in less than 4 bytes
+- Result += input.read(cursor, FrameNum*4);
+- FrameSize = ReadCodedSizeValue(cursor, SizeRead, SizeUnknown);
++ {
++ auto EBMLCodecLength = [](binary buf) {
++ // TODO: use C23 stdc_leading_zeros_uc() as well as GCC/clang __builtin_clz()
++ // __builtin_clz(buf) - 23;
++ if (buf & 0x80)
++ return 1;
++ if (buf & 0x40)
++ return 2;
++ if (buf & 0x20)
++ return 3;
++ if (buf & 0x10)
++ return 4;
++ if (buf & 0x08)
++ return 5;
++ if (buf & 0x04)
++ return 6;
++ if (buf & 0x02)
++ return 7;
++ if (buf & 0x01)
++ return 8;
++ // invalid EBML coded length
++ throw SafeReadIOCallback::EndOfStreamX(0);
++ };
++
++ binary length_buf[8];
++ if (input.read(length_buf, 1) != 1)
++ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ // get the length of the EBML coded value
++ SizeRead = EBMLCodecLength(length_buf[0]);
++ // read remaining needed bytes
++ if (SizeRead > 1 && input.read(&length_buf[1], SizeRead - 1) != SizeRead - 1)
++ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ FrameSize = ReadCodedSizeValue(length_buf, SizeRead, SizeUnknown);
+ if (FrameSize > TotalLacedSize)
+ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ FirstFrameLocation += SizeRead;
+ SizeList[0] = FrameSize;
+- cursor += SizeRead;
+ LastBufferSize -= FrameSize + SizeRead;
+
+ for (Index=1; Index<FrameNum; Index++) {
+- // get the size of the frame
+- SizeRead = LastBufferSize;
+- FrameSize += ReadCodedSizeSignedValue(cursor, SizeRead, SizeUnknown);
++ if (input.read(length_buf, 1) != 1)
++ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ // get the length of the EBML coded value
++ SizeRead = EBMLCodecLength(length_buf[0]);
++ // read remaining needed bytes
++ if (SizeRead > 1 && input.read(&length_buf[1], SizeRead - 1) != SizeRead - 1)
++ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ FrameSize += ReadCodedSizeSignedValue(length_buf, SizeRead, SizeUnknown);
+ if (FrameSize > TotalLacedSize)
+ throw SafeReadIOCallback::EndOfStreamX(0);
++
++ FirstFrameLocation += SizeRead;
+ SizeList[Index] = FrameSize;
+- cursor += SizeRead;
+ LastBufferSize -= FrameSize + SizeRead;
+ }
+
+- FirstFrameLocation += cursor - _tmpBuf;
+-
+ SizeList[Index] = LastBufferSize;
+- delete [] _tmpBuf;
+ break;
++ }
+ case LACING_FIXED:
+ for (Index=0; Index<=FrameNum; Index++) {
+ // get the size of the frame
+--
+2.52.0.windows.1
+
=====================================
contrib/src/matroska/0002-KaxBlock-throw-when-the-EBML-length-difference-gives.patch
=====================================
@@ -0,0 +1,43 @@
+From b4c4a7ef50b3bb8cac30975437c89a377240ef6f Mon Sep 17 00:00:00 2001
+From: Steve Lhomme <robux4 at ycbcr.xyz>
+Date: Tue, 28 Apr 2026 11:30:11 +0200
+Subject: [PATCH 2/2] KaxBlock: throw when the EBML length difference gives a
+ negative frame length
+
+---
+ src/KaxBlock.cpp | 12 ++++++++++--
+ 1 file changed, 10 insertions(+), 2 deletions(-)
+
+diff --git a/src/KaxBlock.cpp b/src/KaxBlock.cpp
+index 5fab622..ddf8b14 100644
+--- a/src/KaxBlock.cpp
++++ b/src/KaxBlock.cpp
+@@ -533,7 +533,11 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
+ for (Index=1; Index<FrameNum; Index++) {
+ // get the size of the frame
+ SizeRead = LastBufferSize;
+- FrameSize += ReadCodedSizeSignedValue(BufferStart + Mem.GetPosition(), SizeRead, SizeUnknown);
++ auto FrameSizeDiff = ReadCodedSizeSignedValue(BufferStart + Mem.GetPosition(), SizeRead, SizeUnknown);
++ if (FrameSizeDiff < 0 && FrameSize <= -FrameSizeDiff)
++ // invalid negative or 0 frame size
++ throw SafeReadIOCallback::EndOfStreamX(SizeRead);
++ FrameSize += FrameSizeDiff;
+ if (!FrameSize || (static_cast<uint32>(FrameSize + SizeRead) > LastBufferSize))
+ throw SafeReadIOCallback::EndOfStreamX(SizeRead);
+ SizeList[Index] = FrameSize;
+@@ -707,7 +711,11 @@ filepos_t KaxInternalBlock::ReadData(IOCallback & input, ScopeMode ReadFully)
+ if (SizeRead > 1 && input.read(&length_buf[1], SizeRead - 1) != SizeRead - 1)
+ throw SafeReadIOCallback::EndOfStreamX(0);
+
+- FrameSize += ReadCodedSizeSignedValue(length_buf, SizeRead, SizeUnknown);
++ auto FrameSizeDiff = ReadCodedSizeSignedValue(length_buf, SizeRead, SizeUnknown);
++ if (FrameSizeDiff < 0 && FrameSize <= -FrameSizeDiff)
++ // invalid negative or 0 frame size
++ throw SafeReadIOCallback::EndOfStreamX(0);
++ FrameSize += FrameSizeDiff;
+ if (FrameSize > TotalLacedSize)
+ throw SafeReadIOCallback::EndOfStreamX(0);
+
+--
+2.52.0.windows.1
+
=====================================
contrib/src/matroska/rules.mak
=====================================
@@ -21,7 +21,8 @@ matroska: libmatroska-$(MATROSKA_VERSION).tar.xz .sum-matroska
$(call pkg_static,"libmatroska.pc.in")
$(APPLY) $(SRC)/matroska/0001-KaxDefines-do-not-allow-infinite-sizes-on-all-Master.patch
$(APPLY) $(SRC)/matroska/0001-KaxBlock-release-read-buffers-on-EndOfStream-error.patch
- $(APPLY) $(SRC)/matroska/0001-KaxBlock-fix-leak-when-reading-EBML-lace-is-aborted.patch
+ $(APPLY) $(SRC)/matroska/0001-KaxBlock-rework-EBML-lacing-sizes-in-SCOPE_PARTIAL_D.patch
+ $(APPLY) $(SRC)/matroska/0002-KaxBlock-throw-when-the-EBML-length-difference-gives.patch
$(MOVE)
.matroska: matroska toolchain.cmake
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/82fd41938885196b345f492e0c411d9194bb3405
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/82fd41938885196b345f492e0c411d9194bb3405
You're receiving this email because of your account on code.videolan.org.
More information about the vlc-commits
mailing list