loadfile, ytdl_hook: don't reject EDL-resolved URLs through playlist

The ytdl wrapper can resolve web links to playlists. This playlist is
passed as big memory:// blob, and will contain further quite normal web
links. When playback of one of these playlist entries starts, ytdl is
called again and will resolve the web link to a media URL again.

This didn't work if playlist entries resolved to EDL URLs. Playback was
rejected with a "potentially unsafe URL from playlist" error. This was
completely weird and unexpected: using the playlist entry directly on
the command line worked fine, and there isn't a reason why it should be
different for a playlist entry (both are resolved by the ytdl wrapper
anyway). Also, if the only EDL URL was added via audio-add or sub-add,
the URL was accessed successfully.

The reason this happened is because the playlist entries were marked as
STREAM_SAFE_ONLY, and edl:// is not marked as "safe". Playlist entries
passed via command line directly are not marked, so resolving them to
EDL worked.

Fix this by making the ytdl hook set load-unsafe-playlists while the
playlist is parsed. (After the playlist is parsed, and before the first
playlist entry is played, file-local options are reset again.) Further,
extend the load-unsafe-playlists option so that the playlist entries are
not marked while the playlist is loaded.

Since playlist entries are already verified, this should change nothing
about the actual security situation.

There are now 2 locations which check load_unsafe_playlists. The old one
is a bit redundant now. In theory, the playlist loading code might not
be the only code which sets these flags, so keeping the old code is
somewhat justified (and in any case it doesn't hurt to keep it).

In general, the security concept sucks (and always did). I can for
example not answer the question whether you can "break" this mechanism
with various combinations of archives, EDL files, playlists files,
compromised sites, and so on. You probably can, and I'm fully aware that
it's probably possible, so don't blame me.
This commit is contained in:
wm4 2019-01-04 13:48:27 +01:00
parent 7fad173cfd
commit d2ef2f98a8
3 changed files with 12 additions and 1 deletions

View File

@ -263,6 +263,11 @@ Playback Control
includes special protocols and anything that doesn't refer to normal files.
Local files and HTTP links on the other hand are always considered safe.
In addition, if a playlist is loaded while this is set, the added playlist
entries are not marked as originating from network or potentially unsafe
location. (Instead, the behavior is as if the playlist entries were provided
directly to mpv command line or ``loadfile`` command.)
Note that ``--playlist`` always loads all entries, so you use that instead
if you really have the need for this functionality.

View File

@ -1429,7 +1429,7 @@ static void play_current_file(struct MPContext *mpctx)
if (mpctx->demuxer->playlist) {
struct playlist *pl = mpctx->demuxer->playlist;
int entry_stream_flags = 0;
if (!pl->disable_safety) {
if (!pl->disable_safety && !mpctx->opts->load_unsafe_playlists) {
entry_stream_flags = STREAM_SAFE_ONLY;
if (mpctx->demuxer->is_network)
entry_stream_flags |= STREAM_NETWORK_ONLY;

View File

@ -644,6 +644,12 @@ function run_ytdl_hook(url)
end
mp.set_property("stream-open-filename", "memory://" .. table.concat(playlist, "\n"))
-- This disables mpv's mushy playlist security code, which will
-- break links that will be resolved to EDL later (because EDL is
-- not considered "safe", and the playlist entries got tagged as
-- network originating due to the playlist redirection).
mp.set_property_native("file-local-options/load-unsafe-playlists", true)
end
else -- probably a video