Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Anton Khirnov]

Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> >
> > Now the question is whether a malicious attacker can craft those two
> > files to get access to anything they shouldn't. I suppose at the very
> > least the attacker can get information that the user opened the file (by
> > adding an asset on an attacker's server) but that will be a danger with
> > any playlists allowing network resources and can be controlled with
> > io_open(). Can you think of any other possible issues?
> >
> 
> Some security considerations:
> 
> - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> does not contain paths/hyperlinks and (b) only those resources
> referenced by the CPL are fetched using the ASSETMAP.
> - the CPL uses XML, which has its own security considerations. For
> example, XML parsing can result in entities being fetched over the
> network, but this is disabled by default in libxml AFAIK.

This is concerning. From a brief glance at libxml2, it seems that you
need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
actually disabling network fetching.
But it is possible I'm misreading the code, so if you or anyone else
understands this better then clarifications are welcome.

> - several elements/attributes of the IMF CPL use URIs as unique
> identifiers. These URIs could conceivably be dereferenced.
> Dereferencing these URIs is however not a requirement and the IMF
> demuxer does not do so.
> - IMF only uses MXF to wrap essence but supports various kinds of
> essence, e.g. Prores and J2K, each with its own security
> considerations

The demuxer should not be concerned about what the user does with the
returned data. Only the behavior of the demuxer itself (and whatever
code it calls) is the question here.

-- 
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Pierre-Anthony Lemieux]

On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> > >
> > > Now the question is whether a malicious attacker can craft those two
> > > files to get access to anything they shouldn't. I suppose at the very
> > > least the attacker can get information that the user opened the file (by
> > > adding an asset on an attacker's server) but that will be a danger with
> > > any playlists allowing network resources and can be controlled with
> > > io_open(). Can you think of any other possible issues?
> > >
> >
> > Some security considerations:
> >
> > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> > does not contain paths/hyperlinks and (b) only those resources
> > referenced by the CPL are fetched using the ASSETMAP.
> > - the CPL uses XML, which has its own security considerations. For
> > example, XML parsing can result in entities being fetched over the
> > network, but this is disabled by default in libxml AFAIK.
>
> This is concerning. From a brief glance at libxml2, it seems that you
> need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> actually disabling network fetching.
> But it is possible I'm misreading the code, so if you or anyone else
> understands this better then clarifications are welcome.

I was referring to entity expansion and the loading of DTDs being
disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
[1-2].

[1] http://xmlsoft.org/html/libxml-parser.html
[2] https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html

>
> > - several elements/attributes of the IMF CPL use URIs as unique
> > identifiers. These URIs could conceivably be dereferenced.
> > Dereferencing these URIs is however not a requirement and the IMF
> > demuxer does not do so.
> > - IMF only uses MXF to wrap essence but supports various kinds of
> > essence, e.g. Prores and J2K, each with its own security
> > considerations
>
> The demuxer should not be concerned about what the user does with the
> returned data. Only the behavior of the demuxer itself (and whatever
> code it calls) is the question here.
>
> --
> 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-request@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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Anton Khirnov]

Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov <anton@khirnov.net> wrote:
> >
> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> > > >
> > > > Now the question is whether a malicious attacker can craft those two
> > > > files to get access to anything they shouldn't. I suppose at the very
> > > > least the attacker can get information that the user opened the file (by
> > > > adding an asset on an attacker's server) but that will be a danger with
> > > > any playlists allowing network resources and can be controlled with
> > > > io_open(). Can you think of any other possible issues?
> > > >
> > >
> > > Some security considerations:
> > >
> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> > > does not contain paths/hyperlinks and (b) only those resources
> > > referenced by the CPL are fetched using the ASSETMAP.
> > > - the CPL uses XML, which has its own security considerations. For
> > > example, XML parsing can result in entities being fetched over the
> > > network, but this is disabled by default in libxml AFAIK.
> >
> > This is concerning. From a brief glance at libxml2, it seems that you
> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> > actually disabling network fetching.
> > But it is possible I'm misreading the code, so if you or anyone else
> > understands this better then clarifications are welcome.
> 
> I was referring to entity expansion and the loading of DTDs being
> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
> [1-2].

Okay then. If nobody has further comments, I will push your latest patch
in a few days.

-- 
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Lynne]

Dec 17, 2021, 3:25 PM by anton@khirnov.net:

> Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
>
>> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov <anton@khirnov.net> wrote:
>> >
>> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
>> > > >
>> > > > Now the question is whether a malicious attacker can craft those two
>> > > > files to get access to anything they shouldn't. I suppose at the very
>> > > > least the attacker can get information that the user opened the file (by
>> > > > adding an asset on an attacker's server) but that will be a danger with
>> > > > any playlists allowing network resources and can be controlled with
>> > > > io_open(). Can you think of any other possible issues?
>> > > >
>> > >
>> > > Some security considerations:
>> > >
>> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
>> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
>> > > does not contain paths/hyperlinks and (b) only those resources
>> > > referenced by the CPL are fetched using the ASSETMAP.
>> > > - the CPL uses XML, which has its own security considerations. For
>> > > example, XML parsing can result in entities being fetched over the
>> > > network, but this is disabled by default in libxml AFAIK.
>> >
>> > This is concerning. From a brief glance at libxml2, it seems that you
>> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
>> > actually disabling network fetching.
>> > But it is possible I'm misreading the code, so if you or anyone else
>> > understands this better then clarifications are welcome.
>>
>> I was referring to entity expansion and the loading of DTDs being
>> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
>> [1-2].
>>
>
> Okay then. If nobody has further comments, I will push your latest patch
> in a few days.
>

I think this shouldn't get merged into 5.0. It would get minimal amount
of fuzzing if it does now, so let's leave it for a later release?
I'd still like to see libuuid being used, we have several uses for it already.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Pierre-Anthony Lemieux]

On Fri, Dec 17, 2021 at 12:54 PM Lynne <dev@lynne.ee> wrote:
>
> Dec 17, 2021, 3:25 PM by anton@khirnov.net:
>
> > Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
> >
> >> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov <anton@khirnov.net> wrote:
> >> >
> >> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> >> > > >
> >> > > > Now the question is whether a malicious attacker can craft those two
> >> > > > files to get access to anything they shouldn't. I suppose at the very
> >> > > > least the attacker can get information that the user opened the file (by
> >> > > > adding an asset on an attacker's server) but that will be a danger with
> >> > > > any playlists allowing network resources and can be controlled with
> >> > > > io_open(). Can you think of any other possible issues?
> >> > > >
> >> > >
> >> > > Some security considerations:
> >> > >
> >> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> >> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> >> > > does not contain paths/hyperlinks and (b) only those resources
> >> > > referenced by the CPL are fetched using the ASSETMAP.
> >> > > - the CPL uses XML, which has its own security considerations. For
> >> > > example, XML parsing can result in entities being fetched over the
> >> > > network, but this is disabled by default in libxml AFAIK.
> >> >
> >> > This is concerning. From a brief glance at libxml2, it seems that you
> >> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> >> > actually disabling network fetching.
> >> > But it is possible I'm misreading the code, so if you or anyone else
> >> > understands this better then clarifications are welcome.
> >>
> >> I was referring to entity expansion and the loading of DTDs being
> >> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
> >> [1-2].
> >>
> >
> > Okay then. If nobody has further comments, I will push your latest patch
> > in a few days.
> >
>
> I think this shouldn't get merged into 5.0. It would get minimal amount
> of fuzzing if it does now, so let's leave it for a later release?

What amount of fuzzing are you looking for? Is there an HLS and/or
DASH equivalent? Happy to put in the work to make it happen.

> I'd still like to see libuuid being used, we have several uses for it already.

libuuid is not an obvious match with the IMF demuxer -- see details in
previous thread. I am happy to work on a UUID refactoring across
libavformat, but this is a separable task with impact on several other
modules.

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer

[Anton Khirnov]

Quoting Lynne (2021-12-17 21:54:14)
> Dec 17, 2021, 3:25 PM by anton@khirnov.net:
> 
> > Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
> >
> >> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov <anton@khirnov.net> wrote:
> >> >
> >> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> >> > > >
> >> > > > Now the question is whether a malicious attacker can craft those two
> >> > > > files to get access to anything they shouldn't. I suppose at the very
> >> > > > least the attacker can get information that the user opened the file (by
> >> > > > adding an asset on an attacker's server) but that will be a danger with
> >> > > > any playlists allowing network resources and can be controlled with
> >> > > > io_open(). Can you think of any other possible issues?
> >> > > >
> >> > >
> >> > > Some security considerations:
> >> > >
> >> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> >> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> >> > > does not contain paths/hyperlinks and (b) only those resources
> >> > > referenced by the CPL are fetched using the ASSETMAP.
> >> > > - the CPL uses XML, which has its own security considerations. For
> >> > > example, XML parsing can result in entities being fetched over the
> >> > > network, but this is disabled by default in libxml AFAIK.
> >> >
> >> > This is concerning. From a brief glance at libxml2, it seems that you
> >> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> >> > actually disabling network fetching.
> >> > But it is possible I'm misreading the code, so if you or anyone else
> >> > understands this better then clarifications are welcome.
> >>
> >> I was referring to entity expansion and the loading of DTDs being
> >> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
> >> [1-2].
> >>
> >
> > Okay then. If nobody has further comments, I will push your latest patch
> > in a few days.
> >
> 
> I think this shouldn't get merged into 5.0. It would get minimal amount
> of fuzzing if it does now, so let's leave it for a later release?
> I'd still like to see libuuid being used, we have several uses for it already.

I don't like this kind of reasoning. Plenty of things get no fuzzing at
all, because they have no tests, yet they go in without complaint.

-- 
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-request@ffmpeg.org with subject "unsubscribe".