[vlc-commits] [Git][videolan/vlc][master] mp4: fix invalid use of undefined pointer state

[Jean-Baptiste Kempf (@jbk)]

Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
9f03d79e by Lyndon Brown at 2022-01-07T07:09:25+00:00
mp4: fix invalid use of undefined pointer state

if the `asprintf()` fails then the pointer is left in an undefined state.

should it fail and the append-to-list and any subsequent loops have managed
to make it through the low-memory condition successfully, then firstly some
code was inevitably going to read the pointer in trying to use the string,
and secondly at some point later when the list of seekpoints is destroyed,
`vlc_seekpoint_Delete()` would be calling `free()` on it.

the original version of this patch simply reset the pointer to null upon
failure - if anything is not designed to cope with a null name then that's
an entirely separate existing bug. however, two other places within this
file that make seekpoints, including the very similar `LoadChapterGpac()`
function, take the action of destroying seekpoints for which they have
failed to allocate the name, and feedback from review requested that same
behaviour here.

- - - - -


1 changed file:

- modules/demux/mp4/mp4.c


Changes:

=====================================
modules/demux/mp4/mp4.c
=====================================
@@ -2293,7 +2293,7 @@ static void LoadChapterGpac( demux_t  *p_demux, MP4_Box_t *p_chpl )
         s->psz_name = strdup( BOXDATA(p_chpl)->chapter[i].psz_name );
         if( s->psz_name == NULL)
         {
-            vlc_seekpoint_Delete( s );;
+            vlc_seekpoint_Delete( s );
             continue;
         }
 
@@ -2312,15 +2312,20 @@ static void LoadChapterGoPro( demux_t *p_demux, MP4_Box_t *p_hmmt )
         for( unsigned i = 0; i < BOXDATA(p_hmmt)->i_chapter_count; i++ )
         {
             seekpoint_t *s = vlc_seekpoint_New();
-            if( s )
-            {
-                if( asprintf( &s->psz_name, "HiLight tag #%u", i+1 ) != -1 )
-                    EnsureUTF8( s->psz_name );
+            if( s == NULL)
+                continue;
 
-                /* HiLights are stored in ms so we convert them to µs */
-                s->i_time_offset = VLC_TICK_FROM_MS( BOXDATA(p_hmmt)->pi_chapter_start[i] );
-                TAB_APPEND( p_sys->p_title->i_seekpoint, p_sys->p_title->seekpoint, s );
+            if( asprintf( &s->psz_name, "HiLight tag #%u", i+1 ) == -1 )
+            {
+                s->psz_name = NULL;
+                vlc_seekpoint_Delete( s );
+                continue;
             }
+
+            EnsureUTF8( s->psz_name );
+            /* HiLights are stored in ms so we convert them to µs */
+            s->i_time_offset = VLC_TICK_FROM_MS( BOXDATA(p_hmmt)->pi_chapter_start[i] );
+            TAB_APPEND( p_sys->p_title->i_seekpoint, p_sys->p_title->seekpoint, s );
         }
 }
 static void LoadChapterApple( demux_t  *p_demux, mp4_track_t *tk )



View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/9f03d79ee48b8cfa3c0c42d3b8a7ee73fc5c9f86

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/9f03d79ee48b8cfa3c0c42d3b8a7ee73fc5c9f86
You're receiving this email because of your account on code.videolan.org.