[vlc-commits] [Git][videolan/vlc][3.0.x] 13 commits: demux: mkv: avoid leaking block on error

Steve Lhomme (@robUx4) gitlab at videolan.org
Sun Mar 15 07:31:09 UTC 2026



Steve Lhomme pushed to branch 3.0.x at VideoLAN / VLC


Commits:
40daa59b by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: avoid leaking block on error

(cherry picked from commit 40c025729ec3ab06204ffad8227533eac43bf625)

- - - - -
998142d1 by Steve Lhomme at 2026-03-15T06:59:37+00:00
Revert "demux:mkv: fix wrong value reset after clean of an array"

This reverts commit d1d41b3f849d4e260c1ec09c5b1bb53db7f8107f.

The i_subpackets value represents the size of the p_subpackets array.
It should not change during the lifetime of that array.

The i_subpacket value is the count of actually filled elements in the array.
When the array is fully loaded we can send the blocks for decoding.

The fix for the hacker one file should be different.

(cherry picked from commit f9500c236765ab64cb00cc4fcbd58db4ec02a193)

- - - - -
a6df2fef by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: only send actual blocks

If i_sub_packet_h/i_sub_packet_h/i_subpacket_size have odd values,
the index may of filled blocks may not cover all the arrays.

We send the blocks that are at the proper place in hope the decoder
can make something of it.

Fixes https://hackerone.com/reports/491495

(cherry picked from commit b01b944250430b4126e89488734a16e48337f88f)

- - - - -
ab3ca2a4 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: check the real_audio_private size based on packed size

(cherry picked from commit 7af48e29453ae08e2ba3236a8250592d06df5869)

- - - - -
6064faf7 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: read the cook/atrac3 blockalign in A_REAL__helper

So that the real_audio_private handling is handled in one location.

(cherry picked from commit b714e22d6be6850a0691a53683a72e9f6be100ad)

- - - - -
4a52a467 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: add a simple byte reader class

Faster than bs_t for bytes and can handle little/big-endian.

(cherry picked from commit 547448a0d1d875bbafbf1c5cbd93fcddcc481a6b) (rebased)
rebased:
- VLC 3 doesn't use vlc_tick_t for matroska timestamps

- - - - -
ee9f8a39 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: parse RealAudio extradata directly in Cook_PrivateTrackData

Using a byte reader to avoid relying on unpacked structures.

Fixes #29420

(cherry picked from commit 7cac9cbe051287bab6763385fdf66a8039b813b8)

- - - - -
b64c88ad by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: move the RealAudio structures in util.cpp

(cherry picked from commit 04f5babb9123123faed26455cb726f2ab811825a)

- - - - -
a478f41e by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: use the coded_frame_size for RA288 block alignment

As done in libavformat.

(cherry picked from commit 216e30ef43c718b640d4d03b31a9e1fd9bb41e19)

- - - - -
763589ba by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: use a C++ vector for the RealAudio sub packets

This is less error-prone.

(cherry picked from commit 49eaa39aacec6c8528993af31b15a732e24cf26e)

- - - - -
86b17529 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: use safer cast to Cook_PrivateTrackData

(cherry picked from commit c9af0b78afddc0ce13e759335e3af1431be08719)

- - - - -
ec28a71c by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: fix potential division by zero

Fixes https://code.videolan.org/videolan/vlc/-/issues/29444

(cherry picked from commit 2c38b23ccd7a1d648599f33be48ad992dd281f4d)

- - - - -
682fd356 by Steve Lhomme at 2026-03-15T06:59:37+00:00
demux: mkv: fix PrivateTrackData not checking errors properly

Cook_PrivateTrackData::Init() was always returning the same value on error or success.

Bug introduced in 7cac9cbe051287bab6763385fdf66a8039b813b8.

Fixes #29523

(cherry picked from commit d51fc15b96817f34d68975e273b28561a84a9868)

- - - - -


4 changed files:

- modules/demux/mkv/matroska_segment_parse.cpp
- modules/demux/mkv/mkv.hpp
- modules/demux/mkv/util.cpp
- modules/demux/mkv/util.hpp


Changes:

=====================================
modules/demux/mkv/matroska_segment_parse.cpp
=====================================
@@ -1877,7 +1877,7 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
             ONLY_FMT(AUDIO);
             uint8_t *p = vars.p_tk->p_extra_data;
 
-            if (vars.p_tk->i_extra_data <= sizeof(real_audio_private))
+            if (vars.p_tk->i_extra_data <= SIZEOF_REALAUDIO_PRIVATE)
                 return false;
 
             if( memcmp( p, ".ra", 3 ) ) {
@@ -1890,39 +1890,29 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
         }
         static void A_REAL__helper (HandlerPayload& vars, uint32_t i_codec) {
             mkv_track_t        * p_tk = vars.p_tk;
-            real_audio_private * priv = (real_audio_private*) p_tk->p_extra_data;
 
             p_tk->fmt.i_codec = i_codec;
 
-            /* FIXME RALF and SIPR */
-            uint16_t version = (uint16_t) hton16(priv->version);
-
-            p_tk->p_sys = new Cook_PrivateTrackData(
-                  hton16( priv->sub_packet_h ),
-                  hton16( priv->frame_size ),
-                  hton16( priv->sub_packet_size )
+            Cook_PrivateTrackData * p_realaudio = new Cook_PrivateTrackData(
+                p_tk->p_extra_data,
+                p_tk->i_extra_data
             );
+            p_tk->p_sys = p_realaudio;
 
-            if( unlikely( !p_tk->p_sys ) )
-                throw std::runtime_error ("p_tk->p_sys is NULL when handling A_REAL/28_8");
+            if( unlikely( !p_realaudio ) )
+                throw std::runtime_error ("Cook_PrivateTrackData is NULL when handling A_REAL/28_8");
 
-            if( unlikely( p_tk->p_sys->Init() ) )
-                throw std::runtime_error ("p_tk->p_sys->Init() failed when handling A_REAL/28_8");
+            if( unlikely( !p_realaudio->Init() ) )
+                throw std::runtime_error ("Cook_PrivateTrackData::Init() failed when handling A_REAL/28_8");
 
-            if( version == 4 )
-            {
-                real_audio_private_v4 * v4 = (real_audio_private_v4*) priv;
-                p_tk->fmt.audio.i_channels = hton16(v4->channels);
-                p_tk->fmt.audio.i_bitspersample = hton16(v4->sample_size);
-                p_tk->fmt.audio.i_rate = hton16(v4->sample_rate);
-            }
-            else if( version == 5 )
-            {
-                real_audio_private_v5 * v5 = (real_audio_private_v5*) priv;
-                p_tk->fmt.audio.i_channels = hton16(v5->channels);
-                p_tk->fmt.audio.i_bitspersample = hton16(v5->sample_size);
-                p_tk->fmt.audio.i_rate = hton16(v5->sample_rate);
-            }
+            if (i_codec == VLC_CODEC_COOK || i_codec == VLC_CODEC_ATRAC3)
+                p_tk->fmt.audio.i_blockalign = p_realaudio->i_subpacket_size;
+            else // VLC_CODEC_RA_288
+                p_tk->fmt.audio.i_blockalign = p_realaudio->coded_frame_size;
+
+            p_tk->fmt.audio.i_rate          = p_realaudio->i_rate;
+            p_tk->fmt.audio.i_bitspersample = p_realaudio->i_bitspersample;
+            p_tk->fmt.audio.i_channels      = p_realaudio->i_channels;
             msg_Dbg(vars.p_demuxer, "%d channels %d bits %d Hz",p_tk->fmt.audio.i_channels, p_tk->fmt.audio.i_bitspersample, p_tk->fmt.audio.i_rate);
 
             fill_extra_data( p_tk, p_tk->fmt.i_codec == VLC_CODEC_RA_288 ? 0 : 78);
@@ -1931,18 +1921,12 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
             if (!A_REAL__is_valid (vars))
                 return;
 
-            real_audio_private * priv = (real_audio_private*) vars.p_tk->p_extra_data;
-            vars.p_tk->fmt.audio.i_blockalign = hton16(priv->sub_packet_size);
-
             A_REAL__helper (vars, VLC_CODEC_COOK);
         }
         S_CASE("A_REAL/ATRC") {
             if (!A_REAL__is_valid (vars))
                 return;
 
-            real_audio_private * priv = (real_audio_private*) vars.p_tk->p_extra_data;
-            vars.p_tk->fmt.audio.i_blockalign = hton16(priv->sub_packet_size);
-
             A_REAL__helper (vars, VLC_CODEC_ATRAC3);
         }
         S_CASE("A_REAL/28_8") {


=====================================
modules/demux/mkv/mkv.hpp
=====================================
@@ -188,7 +188,7 @@ class PrivateTrackData
 {
 public:
     virtual ~PrivateTrackData() {}
-    virtual int32_t Init() { return 0; }
+    virtual bool Init() { return true; }
 };
 
 class mkv_track_t


=====================================
modules/demux/mkv/util.cpp
=====================================
@@ -167,19 +167,18 @@ block_t *MemToBlock( uint8_t *p_mem, size_t i_mem, size_t offset)
 void handle_real_audio(demux_t * p_demux, mkv_track_t * p_tk, block_t * p_blk, vlc_tick_t i_pts)
 {
     uint8_t * p_frame = p_blk->p_buffer;
-    Cook_PrivateTrackData * p_sys = (Cook_PrivateTrackData *) p_tk->p_sys;
+    Cook_PrivateTrackData * p_sys = static_cast<Cook_PrivateTrackData *>(p_tk->p_sys);
     size_t size = p_blk->i_buffer;
 
     if( p_tk->i_last_dts == VLC_TICK_INVALID )
     {
-        for( size_t i = 0; i < p_sys->i_subpackets; i++)
+        for( size_t i = 0; i < p_sys->p_subpackets.size(); i++)
             if( p_sys->p_subpackets[i] )
             {
                 block_Release(p_sys->p_subpackets[i]);
                 p_sys->p_subpackets[i] = NULL;
             }
         p_sys->i_subpacket = 0;
-        p_sys->i_subpackets = 0;
 
         if ( !( p_blk->i_flags & BLOCK_FLAG_TYPE_I) )
         {
@@ -201,45 +200,51 @@ void handle_real_audio(demux_t * p_demux, mkv_track_t * p_tk, block_t * p_blk, v
         {
             size_t i_index = (size_t) p_sys->i_sub_packet_h * i +
                           ((p_sys->i_sub_packet_h + 1) / 2) * (y&1) + (y>>1);
-            if( i_index >= p_sys->i_subpackets )
-                return;
-
-            block_t *p_block = block_Alloc( p_sys->i_subpacket_size );
-            if( !p_block )
+            if( unlikely(i_index >= p_sys->p_subpackets.size()) )
                 return;
 
             if( size < p_sys->i_subpacket_size )
                 return;
 
-            memcpy( p_block->p_buffer, p_frame, p_sys->i_subpacket_size );
-            p_block->i_dts = VLC_TICK_INVALID;
-            p_block->i_pts = VLC_TICK_INVALID;
-            if( !p_sys->i_subpacket )
+            if (likely(p_sys->p_subpackets[i_index] == nullptr))
             {
-                p_tk->i_last_dts =
-                p_block->i_pts = i_pts;
+                // the index was not used
+                block_t *p_block = block_Alloc( p_sys->i_subpacket_size );
+                if( !p_block )
+                    return;
+
+                memcpy( p_block->p_buffer, p_frame, p_sys->i_subpacket_size );
+                p_block->i_dts = VLC_TICK_INVALID;
+                p_block->i_pts = VLC_TICK_INVALID;
+                if( p_sys->i_subpacket == 0 )
+                {
+                    p_tk->i_last_dts =
+                    p_block->i_pts = i_pts;
+                }
+                p_sys->p_subpackets[i_index] = p_block;
             }
 
             p_frame += p_sys->i_subpacket_size;
             size -=  p_sys->i_subpacket_size;
 
             p_sys->i_subpacket++;
-            p_sys->p_subpackets[i_index] = p_block;
         }
     }
     else
     {
         /*TODO*/
     }
-    if( p_sys->i_subpacket == p_sys->i_subpackets )
+    if( p_sys->i_subpacket == p_sys->p_subpackets.size() )
     {
-        for( size_t i = 0; i < p_sys->i_subpackets; i++)
+        for( size_t i = 0; i < p_sys->p_subpackets.size(); i++)
         {
-            send_Block( p_demux, p_tk, p_sys->p_subpackets[i], 1, 0 );
-            p_sys->p_subpackets[i] = NULL;
+            if (likely(p_sys->p_subpackets[i]))
+            {
+                send_Block( p_demux, p_tk, p_sys->p_subpackets[i], 1, 0 );
+                p_sys->p_subpackets[i] = NULL;
+            }
         }
         p_sys->i_subpacket = 0;
-        p_sys->i_subpackets = 0;
     }
 }
 
@@ -384,27 +389,98 @@ void send_Block( demux_t * p_demux, mkv_track_t * p_tk, block_t * p_block, unsig
     es_out_Send( p_demux->out, p_tk->p_es, p_block);
 }
 
-int32_t Cook_PrivateTrackData::Init()
+struct real_audio_private
+{
+    uint32_t fourcc;
+    uint16_t version;
+    uint16_t unknown1;
+    uint8_t  unknown2[12];
+    uint16_t unknown3;
+    uint16_t flavor;
+    uint32_t coded_frame_size;
+    uint32_t unknown4[3];
+    uint16_t sub_packet_h;
+    uint16_t frame_size;
+    uint16_t sub_packet_size;
+    uint16_t unknown5;
+};
+
+struct real_audio_private_v4
 {
-    i_subpackets = (size_t) i_sub_packet_h * (size_t) i_frame_size / (size_t) i_subpacket_size;
-    p_subpackets = static_cast<block_t**> ( calloc(i_subpackets, sizeof(block_t*)) );
+    real_audio_private header;
+    uint16_t sample_rate;
+    uint16_t unknown;
+    uint16_t sample_size;
+    uint16_t channels;
+};
 
-    if( unlikely( !p_subpackets ) )
+
+struct real_audio_private_v5
+{
+    real_audio_private header;
+    uint32_t unknown1;
+    uint16_t unknown2;
+    uint16_t sample_rate;
+    uint16_t unknown3;
+    uint16_t sample_size;
+    uint16_t channels;
+};
+
+bool Cook_PrivateTrackData::Init()
+{
+    // real_audio_private
+    bytes.skip(4);                      // fourcc
+
+    /* FIXME RALF and SIPR */
+    uint16_t version = bytes.GetBE16(); // version
+    bytes.skip(2);                      // unknown1
+    bytes.skip(12);                     // unknown2
+    bytes.skip(2);                      // unknown3
+    bytes.skip(2);                      // flavor
+    coded_frame_size = bytes.GetBE32(); // coded_frame_size
+    bytes.skip(3*4);                    // unknown4
+    i_sub_packet_h   = bytes.GetBE16(); // sub_packet_h
+    i_frame_size     = bytes.GetBE16(); // frame_size
+    i_subpacket_size = bytes.GetBE16(); // sub_packet_size
+    bytes.skip(2);                      // unknown5
+
+    if ( i_subpacket_size == 0 )
+        return false;
+
+    if( version == 4 )
     {
-        i_subpackets = 0;
-        return 1;
+        // real_audio_private_v4
+        i_rate          = bytes.GetBE16(); // sample_rate
+        bytes.skip(2);                     // unknown
+        i_bitspersample = bytes.GetBE16(); // sample_size
+        i_channels      = bytes.GetBE16(); // channels
+    }
+    else if( version == 5 )
+    {
+        // real_audio_private_v5
+        bytes.skip(4);                     // unknown1
+        bytes.skip(2);                     // unknown2
+        i_rate          = bytes.GetBE16(); // sample_rate
+        bytes.skip(2);                     // unknown2
+        i_bitspersample = bytes.GetBE16(); // sample_size
+        i_channels      = bytes.GetBE16(); // channels
     }
+    else
+        return false;
+    if (bytes.hasErrors())
+        return false;
 
-    return 0;
+    size_t i_subpackets = (size_t) i_sub_packet_h * (size_t) i_frame_size / (size_t) i_subpacket_size;
+    p_subpackets.resize(i_subpackets);
+
+    return true;
 }
 
 Cook_PrivateTrackData::~Cook_PrivateTrackData()
 {
-    for( size_t i = 0; i < i_subpackets; i++ )
+    for( size_t i = 0; i < p_subpackets.size(); i++ )
         if( p_subpackets[i] )
             block_Release( p_subpackets[i] );
-
-    free( p_subpackets );
 }
 
 static inline void fill_wvpk_block(uint16_t version, uint32_t block_samples, uint32_t flags,


=====================================
modules/demux/mkv/util.hpp
=====================================
@@ -37,59 +37,81 @@ block_t *WEBVTT_Repack_Sample(block_t *p_block, bool b_webm = false,
 void send_Block( demux_t * p_demux, mkv_track_t * p_tk, block_t * p_block, unsigned int i_number_frames, vlc_tick_t i_duration );
 int UpdatePCR( demux_t * p_demux );
 
-
-struct real_audio_private
+class ByteReader
 {
-    uint32_t fourcc;
-    uint16_t version;
-    uint16_t unknown1;
-    uint8_t  unknown2[12];
-    uint16_t unknown3;
-    uint16_t flavor;
-    uint32_t coded_frame_size;
-    uint32_t unknown4[3];
-    uint16_t sub_packet_h;
-    uint16_t frame_size;
-    uint16_t sub_packet_size;
-    uint16_t unknown5;
-};
+public:
+    ByteReader(const uint8_t *reader_, size_t reader_len_):
+        reader(reader_), reader_left(reader_len_) {}
 
-struct real_audio_private_v4
-{
-    real_audio_private header;
-    uint16_t sample_rate;
-    uint16_t unknown;
-    uint16_t sample_size;
-    uint16_t channels;
-};
+    bool skip(size_t bytes) {
+        if (error) return false;
+        if (reader_left < bytes) {
+            reader_left = 0;
+            error = true;
+            return false;
+        }
+        reader_left -= bytes;
+        reader += bytes;
+        return true;
+    }
 
+    uint16_t GetBE16() {
+        if (error) return 0;
+        if (reader_left < 2) {
+            reader_left = 0;
+            error = true;
+            return 0;
+        }
+        uint16_t v = GetWBE(reader);
+        reader_left -= 2;
+        reader += 2;
+        return v;
+    }
 
-struct real_audio_private_v5
-{
-    real_audio_private header;
-    uint32_t unknown1;
-    uint16_t unknown2;
-    uint16_t sample_rate;
-    uint16_t unknown3;
-    uint16_t sample_size;
-    uint16_t channels;
+    uint32_t GetBE32() {
+        if (error) return 0;
+        if (reader_left < 4) {
+            reader_left = 0;
+            error = true;
+            return 0;
+        }
+        uint16_t v = GetDWBE(reader);
+        reader_left -= 4;
+        reader += 4;
+        return v;
+    }
+
+    bool hasErrors() const { return error; }
+
+private:
+    const uint8_t *reader;
+    size_t        reader_left;
+    bool          error = false;
 };
 
+#define SIZEOF_REALAUDIO_PRIVATE  (4+2+2+12+2+2+4+(3*4)+2+2+2+2)
+
 class Cook_PrivateTrackData : public PrivateTrackData
 {
 public:
-    Cook_PrivateTrackData(uint16_t sph, uint16_t fs, uint16_t sps):
-        i_sub_packet_h(sph), i_frame_size(fs), i_subpacket_size(sps),
-        p_subpackets(NULL), i_subpackets(0), i_subpacket(0){}
+    Cook_PrivateTrackData(const uint8_t *reader, size_t reader_len):
+        bytes(reader, reader_len) {}
     ~Cook_PrivateTrackData();
-    int32_t Init();
+    bool Init() override;
 
+    uint32_t coded_frame_size;
     uint16_t i_sub_packet_h;
     uint16_t i_frame_size;
     uint16_t i_subpacket_size;
-    block_t  **p_subpackets;
-    size_t   i_subpackets;
-    size_t   i_subpacket;
+
+    uint16_t i_rate;
+    uint16_t i_bitspersample;
+    uint16_t i_channels;
+
+    std::vector<block_t *> p_subpackets;
+    size_t   i_subpacket = 0;
+private:
+    ByteReader bytes;
 };
 
 block_t * packetize_wavpack( const mkv_track_t &, uint8_t *, size_t);



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9886bd129efaefc19f0bff200a0c68fbf42990a6...682fd356db0e707c4d643f5fba9ba683510eb133

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9886bd129efaefc19f0bff200a0c68fbf42990a6...682fd356db0e707c4d643f5fba9ba683510eb133
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list