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

[Michael Niedermayer]

On Tue, May 09, 2023 at 08:19:36AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-05-09 00:35:08)
> > 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.
> > > > 
> > > > 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?
> > > 
> > > DASH is usually transferred over HTTP where file extensions are of minor
> > > interest, the relevant type information is in the Mime-Type header.
> > 
> > would anyone be opposed to return 0 from dash_probe() when
> > both the mime_type and the extension are wrong ?
> 
> I would.
> 
> probe() is for probing, not implementing security policies. IMO trying
> to fix security issues at the wrong layer will only lead to more
> confusion, more complexity, and LESS security.

YES i agree, probe is not for security policies

Its for probing but IMHO
If you have a
taxreport.pdf that parses correctly as jar and installs jRAT if you execute it
Then it would be valid for probe() to identify this as type exploit instead
of type jar. And doing so would be more secure.

This is really more along the line of thought here for hls too.
a file with avi/mkv/mov/mxf/mpg/mp4 extension is not a hls playlist
Could someone have added that extension by mistake, yes
similarly your jar file could be named .pdf by mistake. But thats not 
a good default assumtation and i dont think anyone would assume that
by default.

thx

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

If the United States is serious about tackling the national security threats 
related to an insecure 5G network, it needs to rethink the extent to which it
values corporate profits and government espionage over security.-Bruce Schneier
-------------- 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/20230509/86d645e1/attachment.sig>