Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
       [not found]       ` <CAF_7JxCCye8oAdjvqtPY4aUMKmgfY6poa4NtyKw3X-OkCsfRmg@mail.gmail.com>
@ 2021-12-15 20:20         ` Anton Khirnov
  2021-12-15 20:41           ` Pierre-Anthony Lemieux
  0 siblings, 1 reply; 6+ messages in thread
From: Anton Khirnov @ 2021-12-15 20:20 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
  2021-12-15 20:20         ` [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer Anton Khirnov
@ 2021-12-15 20:41           ` Pierre-Anthony Lemieux
  2021-12-17 14:25             ` Anton Khirnov
  0 siblings, 1 reply; 6+ messages in thread
From: Pierre-Anthony Lemieux @ 2021-12-15 20:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
  2021-12-15 20:41           ` Pierre-Anthony Lemieux
@ 2021-12-17 14:25             ` Anton Khirnov
  2021-12-17 20:54               ` Lynne
  0 siblings, 1 reply; 6+ messages in thread
From: Anton Khirnov @ 2021-12-17 14:25 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
  2021-12-17 14:25             ` Anton Khirnov
@ 2021-12-17 20:54               ` Lynne
  2021-12-17 21:09                 ` Pierre-Anthony Lemieux
  2021-12-20  9:48                 ` Anton Khirnov
  0 siblings, 2 replies; 6+ messages in thread
From: Lynne @ 2021-12-17 20:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
  2021-12-17 20:54               ` Lynne
@ 2021-12-17 21:09                 ` Pierre-Anthony Lemieux
  2021-12-20  9:48                 ` Anton Khirnov
  1 sibling, 0 replies; 6+ messages in thread
From: Pierre-Anthony Lemieux @ 2021-12-17 21:09 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
  2021-12-17 20:54               ` Lynne
  2021-12-17 21:09                 ` Pierre-Anthony Lemieux
@ 2021-12-20  9:48                 ` Anton Khirnov
  1 sibling, 0 replies; 6+ messages in thread
From: Anton Khirnov @ 2021-12-20  9:48 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-20  9:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20211213054336.19783-1-pal@sandflow.com>
     [not found] ` <163947788654.13029.10298345109744593134@lain.red.khirnov.net>
     [not found]   ` <CAF_7JxCxrb0fBNzH_nr=nv8qgHDgMLvAk8WFs7V5dnVfVT_ofA@mail.gmail.com>
     [not found]     ` <163950889025.13443.18056291142265665105@lain.red.khirnov.net>
     [not found]       ` <CAF_7JxCCye8oAdjvqtPY4aUMKmgfY6poa4NtyKw3X-OkCsfRmg@mail.gmail.com>
2021-12-15 20:20         ` [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer Anton Khirnov
2021-12-15 20:41           ` Pierre-Anthony Lemieux
2021-12-17 14:25             ` Anton Khirnov
2021-12-17 20:54               ` Lynne
2021-12-17 21:09                 ` Pierre-Anthony Lemieux
2021-12-20  9:48                 ` Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git