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

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v10 1/2] avformat/imf: Demuxer
Date: Mon, 20 Dec 2021 10:48:02 +0100
Message-ID: <163999368282.13029.6411964162989610080@lain.red.khirnov.net> (raw)
In-Reply-To: <Mr9A74---3-2@lynne.ee>

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