Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On Wed, May 10, 2023 at 08:44:31AM +0200, Tobias Rapp wrote: > On 09/05/2023 22:44, Michael Niedermayer wrote: > > > On Tue, May 09, 2023 at 08:19:36AM +0200, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2023-05-09 00:35:08) > > > > [...] > > > > 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 > > > > [...] > > But if the application expects a HLS playlist would it really be a problem > if the input file ends with .avi or some other extension? The probe function > just doesn't know what the application expects. Expectation and actual input > type are aligned after probe. if the application is just for hls then sure you are correct but then also why would the application even run probe ? it would be a waste of cpu cycles from another direction, i think this viewpoint while true is too much going to special case optimization. Maybe we can factor the hls probe in 2 cases. One would be unambiguous hls (mime/extension content all matching) ambigous hls (mime/extension not correct or maybe some very odd URLS in it but otherwise valid hls) This would not loose any detected files but would give more details to user apps and users to make the choice. a user or app could then simply include or not include the ambigous hls in the format whitelist or blacklist This would also not complicate the API but just use the existing features thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On 09/05/2023 22:44, Michael Niedermayer wrote: On Tue, May 09, 2023 at 08:19:36AM +0200, Anton Khirnov wrote: Quoting Michael Niedermayer (2023-05-09 00:35:08) [...] 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 [...] But if the application expects a HLS playlist would it really be a problem if the input file ends with .avi or some other extension? The probe function just doesn't know what the application expects. Expectation and actual input type are aligned after probe. Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
> > > > > ---
> > > > > 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
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On Tue, May 09, 2023 at 09:35:09AM +0200, Tobias Rapp wrote: > On 09/05/2023 08:19, 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: > > > > [...] > > > > 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. > > I agree that probing should be unrelated to the actual format selection > policy. > > > > example: a crafted image.jpeg uploaded somewhere is played as dash. > > > or am i missing something that would stop that ? > The player application could exclude the dash format (and other playlist > formats) from the format_whitelist I guess? That would push the problem down to every application which is really not a very good solution Its even worse because every player than needs to also know which format is a playlist format. Including all future ones and then also if the user minds them being disabled completely thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "You are 36 times more likely to die in a bathtub than at the hands of a terrorist. Also, you are 2.5 times more likely to become a president and 2 times more likely to become an astronaut, than to die in a terrorist attack." -- Thoughty2 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On 09/05/2023 08:19, 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: [...] 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. I agree that probing should be unrelated to the actual format selection policy. example: a crafted image.jpeg uploaded somewhere is played as dash. or am i missing something that would stop that ? The player application could exclude the dash format (and other playlist formats) from the format_whitelist I guess? The alternative for the player application if it doesn't need to depend on the system installation of FFmpeg libraries would be to exclude unwanted formats at compilation time. Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
> > > > ---
> > > > 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.
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
> > > ---
> > > 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 ?
Here both the server says its not dash as well as the extension not being the
standard for dash
example: a crafted image.jpeg uploaded somewhere is played as dash.
or am i missing something that would stop that ?
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Never trust a computer, one day, it may think you are the virus. -- Compn
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On Mon, May 8, 2023 at 10:11 AM Michael Niedermayer
wrote:
>
> 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
> > > > ---
> > > > 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
That presumes that "readme.txt":
(a) is detected as a playlist
(b) is a valid XML file (in the case of IMF or dash playlists)
(c) is structured to be interpreted as a playlist by the demuxer
This is unlikely to happen with a random "readme.txt". It sounds like
we are specifically trying to protect against a maliciously-crafted
"readme.txt".
If so, could enforcing, by default, same origin mitigate risks?
>
> 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.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
> > > ---
> > > 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.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
On Mon, May 8, 2023 at 7:05 AM 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
> >> ---
> >> 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.
>
> 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?
Yes, the IMF playlist, for example, is only allowed to reference MXF files:
https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/imfdec.c#L393
>
> Regards, Tobias
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
---
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.
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?
Regards, Tobias
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
---
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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
Quoting Michael Niedermayer (2023-05-06 15:25:01) > 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 I heavily dislike this. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avformat/dashdec: fail on probing non mpd file extension
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
---
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;
}
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
