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

From: Pierre-Anthony Lemieux <pal@sandflow.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
Date: Fri, 17 Dec 2021 13:09:12 -0800
Message-ID: <CAF_7JxAEAkG4GyChkibykhLP3DfhwF7rBmV5rPvQcNToXOfBKQ@mail.gmail.com> (raw)
In-Reply-To: <Mr9A74---3-2@lynne.ee>

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