GNOME Bugzilla – Bug 725134
hlsdemux: null pointer deref if media playlist contains no media
Last modified: 2014-03-01 15:35:43 UTC
hlsdemux causes a null pointer dereference if it fails to download the media playlist. The gst_hls_demux_update_playlist function assumes that demux->client->current->files is valid and tries to access the last item of the files list when caching the first three fragments at start up. This bug can be reproduced by creating a master m3u8 file that contains one media playlist that does not exist. For example: #EXTM3U #EXT-X-VERSION:4 #EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=617508, CODECS="avc1.42001f mp4a.40.2", RESOLUTION=540x352 /404.m3u8
Created attachment 270257 [details] [review] Proposed patch to fix bug725134
I think it should just fail when it fails to fetch the playlist, and not even go to any of the code that currently does the NULL pointer dereference.
A very good point. This issue came to light when trying to resolve bug725137. Having done some more digging, the problem is not that the playlist fails to load, but that the playlist does not contain media assets. bug725137 is causing the wrong URL to be generated, and the CDN provider is returning the master playlist when presented with this wrong URL. I think this bug still stands however, as it is possible that an incorrectly configured HLS server might provide a playlist without media assets. This shouldn't cause a segfault. I will update the bug description.
This bug can be reproduced by creating an m3u8 file called bug725140.m3u8 that contains: #EXTM3U #EXT-X-VERSION:4 #EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=617508, CODECS="avc1.42001f mp4a.40.2", RESOLUTION=540x352 bug725140.m3u8
True, nonetheless I think additionally a change is necessary to let hlsdemux error out immediately if such problems are found that can't be recovered from.
Do you want to update your patch for that? Otherwise I'll do it on top of your patch
Shouldn't gst_m3u8_client_update() return FALSE if that happens, and then we go out of gst_hls_demux_update_playlist() directly? The duration change is nonetheless necessary.
Created attachment 270622 [details] [review] second version of patch Sorry for the delay in answering the comments - it's never a good day when it takes three OS re-installs to be able to test a three line patch! As suggested, this version returns an error if the media playlist does not contain any media segments.
Thanks for updating the patch :) commit b7ef52c515d9de53a08c450eef84492ba79796b3 Author: Alex Ashley <bugzilla@ashley-family.net> Date: Tue Feb 25 11:45:46 2014 +0000 hlsdemux: Segfaults if playlist has no media files hlsdemux causes a null pointer dereference if the media playlist does not contain any media files. The gst_m3u8_client_get_duration function assumes that demux->client->current->files is valid when computing duration. gst_m3u8_client_update needed to be modified to check for the case of downloading an M3U8 file that doesn't contain any media files, and returning an error to gsthlsdemux.c This bug can be reproduced by creating a master m3u8 file that contains one media playlist that points back to the master m3u8 file. For example create a file called bug725134.m3u8: #EXTM3U #EXT-X-VERSION:4 #EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1251135, CODECS="avc1.42001f mp4a.40.2", RESOLUTIO bug725134.m3u8 https://bugzilla.gnome.org/show_bug.cgi?id=725134