[FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension

[Michael Niedermayer]

On Mon, May 08, 2023 at 04:05:40PM +0200, Tobias Rapp wrote:
> On 08/05/2023 14:00, James Almer wrote:
> 
> > On 5/6/2023 10:25 AM, Michael Niedermayer wrote:
> > > Its unexpected that a .avi or other "standard" file turns into a
> > > playlist.
> > > The goal of this patch is to avoid this unexpected behavior and possible
> > > privacy or security differences.
> > > 
> > > This is similar to the same change to hls
> > > 
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >   libavformat/dashdec.c | 11 +++++++----
> > >   1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> > > index 29d4680c68..294e14150d 100644
> > > --- a/libavformat/dashdec.c
> > > +++ b/libavformat/dashdec.c
> > > @@ -2336,10 +2336,13 @@ static int dash_probe(const AVProbeData *p)
> > >           av_stristr(p->buf, "dash:profile:isoff-live:2011") ||
> > >           av_stristr(p->buf, "dash:profile:isoff-live:2012") ||
> > >           av_stristr(p->buf, "dash:profile:isoff-main:2011") ||
> > > -        av_stristr(p->buf, "3GPP:PSS:profile:DASH1")) {
> > > -        return AVPROBE_SCORE_MAX;
> > > -    }
> > > -    if (av_stristr(p->buf, "dash:profile")) {
> > > +        av_stristr(p->buf, "3GPP:PSS:profile:DASH1") ||
> > > +        av_stristr(p->buf, "dash:profile")) {
> > > +        if (!av_match_ext(p->filename, "mpd")) {
> > > +            av_log(NULL, AV_LOG_ERROR, "Not detecting dash with non
> > > standard extension\n");
> > > +            return 0;
> > > +        }
> > > +
> > >           return AVPROBE_SCORE_MAX;
> > >       }
> > 
> > Failing because it didn't match an extensions sort of goes against the
> > point of probing, which even has a low score return value that's
> > basically "it matched extension" as a sort of last resort.

True


> > 
> > I'd say wrap this in a FF_COMPLIANCE_STRICT check (since i assume the
> > spec does state mpd must be the extension), but i think we have no
> > access to the AVFormatContext here?

Thats not what this was intended to do.

The whole idea is more like a user clicking on a readme.txt and not
expecting that to downloade a list of URLs in it because it happens to be a
valid list or URLs

The problem here is the information available to the user suggests one thing
but the action of the user of opening this file does something different, something
unexpected

Thats not an issue if the difference is between 2 of 1000 similar formats
but If the user believes the format cannot open random local and remote 
URLs but is just a single monolithic file and then when she clicks it
does open other things without the user even ever knowing. That is not
ideal.


> 
> DASH is usually transferred over HTTP where file extensions are of minor
> interest, the relevant type information is in the Mime-Type header.

yes, true


> 
> I think we already have the "format_whitelist" API for applications that
> want to restrict the list of formats when loading a file from untrusted
> sources?



[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20230508/348c4159/attachment.sig>