[vlc-devel] [PATCH 2/2] playlist/fetcher: do not ignore metadata scope when downloading art

[Pierre Ynard]

> - Exactly what is it that I am *"now denying"*?

That there can be another interpretation of this flag than yours.

> I honestly cannot read `--no-network-metadata-access` and see your
> point, if it was meant to block *metadata lookups*; why is it not
> named `--no-network-metadata-lookup`?

I don't know what question you asked your friends, but when I read the
first-run dialog of the Qt interface, it seems clear to me that it's
about privacy and foremost local media files. I don't care about the
name of the option, there is just no feature in blocking access that
causes no privacy or trust concern, and it should be the object of no
configuration option. No benefit, no feature, no point for an option.

> > However this is starting to be a different issue from privacy and
> > querying metadata, it's an issue of blocking untrusted input and
> > attack vectors. And I don't think that this was the concern in
> > mind when the feature was designed, so one could say that you're
> > diverting the real purpose of a loosely related feature to fix a
> > proof-of-concept exploit. It also seems that the patch is wrongly
> > coupling the file caching technical optimization concern with the
> > privacy concern.
>
> Could you elaborate this point?

An attacker can distribute a crafted file or playlist including a
malicious art URL or playback item URL aimed at one of the following:
remote notification (deliberate privacy leak), unwanted access to
harmful content, DoS, malicious access to a local resource hidden from
the attacker (private network, local file...). This is much broader than
the semantics question of fetching art or not given the value of that
flag.

Also, the patch only takes effect inside DownloadArt(). If access should
be controlled for privacy reasons, all access should be controlled, not
just access for the technical benefit of caching. That's on top of the
existing code already making a confusion between file:// scheme, local,
and in-cache.

> > I'm not aware of any protection in other demuxers from untrusted art
> > URLs, nor from privacy leaks they might incur.
>
> Demuxers generally do not download any metadata, that happens in the
> affected code-path which the original patch modified.
>
> As far as I know, we have no such demuxer.

Protections as in, one approach would be that demuxers should evaluate
external references that they extract and refrain from passing unsafe
ones to the core. (Another approach would be that demuxers don't have
enough context to make that decision themselves and the core should do
it instead.)

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."