Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 27.01.2016 09:05, Anton Khirnov wrote:
> Quoting Andreas Cadhalpun (2016-01-27 01:18:23)
>> Could you explain in more detail what problem that would cause?
>> The whitelist should simply be passed from http to tcp in that case.
> 
> You have to go over all the (de)muxers that call the protocol layer
> directly and all the protocols and ensure that those
> restrictions are passed through correctly. That is a somewhat nontrivial
> amount of work. Not to mention that we first have to design those
> restrictions. That's also a not completely obvious step, especially if,
> as you're suggesting below, they should somehow be format-dependent.
> 
> Frankly, I don't have that much free time on my hands to do all that
> quickly. Or do you volunteer?

Fortunately Michael already did that [1][2].

>> I think that applications wanting this behavior should explicitly enable
>> it. To ease the transition period, one could set the protocol_whitelist
>> to something sensible for this case, e.g. 'file,http,https,tcp', when the
>> filename has the extension of a playlist.
> 
> That implies the whitelist is format-dependent, which is already rather
> nontrivial.

Yes, that's unfortunately true. But one could e.g. append some protocols
to the whitelist (and emit a warning that this is a potential security
problem and will thus stop working unless the protocol_whitelist is set)
in the hls demuxer if the url is http/https (and the whitelist the default
for the file protocol), so that this use case would keep working for a
transition period.

>> Then just add quick hacks preventing the worst problems.
>> I'd rather not have functionality hastily removed due to security
>> concerns, that could be easily worked around until a proper solution
>> is found.
> 
> I don't really understand your position -- on one hand you don't want to
> remove "useful" functionality, even though the concat protocol is IMO
> borderline-useless. OTOH you are quite fine with breaking applications
> by adding restrictive defaults.

My main point is that removing the concat protocol is not a proper fix
for this problem. Breaking use-cases is never good and should happen
with a transition period.

On 28.01.2016 10:26, Anton Khirnov wrote:
> Quoting Andreas Cadhalpun (2016-01-27 01:15:07)
>> I think that at the very least the hls demuxer should always reject protocols
>> internal to libavformat, like concat, as those simply do not belong into a 
>> hls
>> playlist.
>>
> 
> I find it rather strange how you're talking about solving the general
> problem in a proper way, but here you say that specifically the hls
> demuxer should behave in some special way.
> 
> The general problem is not specific to hls, and can just as well happen
> with any future playlist-based demuxer, so the solution should not be
> hls-specific.

Yes, but that there is a general problem doesn't mean that there is no
problem in the hls demuxer.
A 'concat:' line just doesn't make sense in a hls playlist and thus shouldn't
be accepted by libavformat, as it also isn't accepted by other players.

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-January/188238.html
2: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-January/188239.html

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

Quoting Andreas Cadhalpun (2016-01-27 01:15:07)
> On 26.01.2016 09:52, Luca Barbato wrote:
> > On 26/01/16 01:02, Andreas Cadhalpun wrote:
> >> On 22.01.2016 00:34, Luca Barbato wrote:
> >>> The ways to fix the specific problem problem:
> >>>
> >>> - provide a blacklist/whitelist option in hls (from me, first
> >>> solution)-> Anton disliked it since it is too specific,
> >>
> >> However, the hls demuxer should somehow validate the URLs it's reading.
> >> Your suggestion would be one way to do that, others are possible as well.
> > 
> > As long the solution is not brittle I guess there will be agreement on
> > it, in the mean time I'd disable concat.
> 
> I think that at the very least the hls demuxer should always reject protocols
> internal to libavformat, like concat, as those simply do not belong into a hls
> playlist.
> 

I find it rather strange how you're talking about solving the general
problem in a proper way, but here you say that specifically the hls
demuxer should behave in some special way.

The general problem is not specific to hls, and can just as well happen
with any future playlist-based demuxer, so the solution should not be
hls-specific.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

Quoting Andreas Cadhalpun (2016-01-27 01:18:23)
> On 26.01.2016 19:42, Anton Khirnov wrote:
> > Quoting Andreas Cadhalpun (2016-01-26 01:02:04)
> >> On 22.01.2016 13:37, Anton Khirnov wrote:
> >>> Just so it's clear what we're talking about, what is "properly" for you?
> >>
> >> That would be a more or less general mechanism, which would have prevented
> >> the information leak in the hls demuxer by default. It should also prevent
> >> any such possible problems in other demuxers.
> >> This could be done by implementing a protocol_whitelist with sensible
> >> defaults.
> > 
> > That was my first idea as well. Turns out, it's rather nontrivial to
> > implement, due to the fact that protocols allow nesting (e.g. http over
> > tcp).
> 
> Could you explain in more detail what problem that would cause?
> The whitelist should simply be passed from http to tcp in that case.

You have to go over all the (de)muxers that call the protocol layer
directly and all the protocols and ensure that those
restrictions are passed through correctly. That is a somewhat nontrivial
amount of work. Not to mention that we first have to design those
restrictions. That's also a not completely obvious step, especially if,
as you're suggesting below, they should somehow be format-dependent.

Frankly, I don't have that much free time on my hands to do all that
quickly. Or do you volunteer?

> 
> > It would also require the calling programs to have quite a lot of
> > knowledge about the libav protocol layer internals.
> 
> Most programs should be able to work with the default.
> 
> >>> And what do you see as "the underlying problem"?
> >>
> >> I think that is libavformat mixing local and remote data. If it wouldn't
> >> to do that by default, such information leaks shouldn't be possible.
> >>
> > 
> > Opening a local playlist that references remote streams is quite common,
> 
> Though it is a lot less common than simply opening a local file.
> 
> > so making this behaviour forbidden by default is likely to break many
> > applications.
> 
> I think that applications wanting this behavior should explicitly enable
> it. To ease the transition period, one could set the protocol_whitelist
> to something sensible for this case, e.g. 'file,http,https,tcp', when the
> filename has the extension of a playlist.

That implies the whitelist is format-dependent, which is already rather
nontrivial.

> 
> > So while I fully agree that the concat protocol is not the root problem,
> > it is the source of its most significat symptom and removing it will
> > prevent the worst leaks.
> 
> That could also be done by disabling it by default, i.e. unless an option
> to enable it is given. That way people using it could at least continue
> to do so after setting said option.
> 
> > Meanwhile, we will have time to introduce the
> > infrastructure to fix this properly (which I think I already started,
> > but finishing it is, as said above, nontrivial).
> > And I'd really rather not design new APIs at gunpoint from pressing
> > security bugs.
> 
> Then just add quick hacks preventing the worst problems.
> I'd rather not have functionality hastily removed due to security
> concerns, that could be easily worked around until a proper solution
> is found.

I don't really understand your position -- on one hand you don't want to
remove "useful" functionality, even though the concat protocol is IMO
borderline-useless. OTOH you are quite fine with breaking applications
by adding restrictive defaults.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Peter B.]

On 01/27/2016 01:21 PM, Luca Barbato wrote:
> On 27/01/16 12:10, Peter B. wrote:
>> I'm a user of "concat" for creating lossy access-copies of segmented
>> lossless archival files.
>> This is done in one step, integrated in an automated mass-digitization
>> workflow.
> You can use hls as an editlist. It is much flexible incidentally.

Will read up on this "hls" feature.

Thanks for the tip :)

Regards,
Pb

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 27/01/16 12:10, Peter B. wrote:
> 
> On 01/27/2016 09:05 AM, Anton Khirnov wrote:
>> I don't really understand your position -- on one hand you don't want to
>> remove "useful" functionality, even though the concat protocol is IMO
>> borderline-useless. OTOH you are quite fine with breaking applications
>> by adding restrictive defaults.
> 
> I'm a user of "concat" for creating lossy access-copies of segmented
> lossless archival files.
> This is done in one step, integrated in an automated mass-digitization
> workflow.

You can use hls as an editlist. It is much flexible incidentally.

lu

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Peter B.]

On 01/27/2016 09:05 AM, Anton Khirnov wrote:
> I don't really understand your position -- on one hand you don't want to
> remove "useful" functionality, even though the concat protocol is IMO
> borderline-useless. OTOH you are quite fine with breaking applications
> by adding restrictive defaults.

I'm a user of "concat" for creating lossy access-copies of segmented
lossless archival files.
This is done in one step, integrated in an automated mass-digitization
workflow.

Following the discussion, I wanted to ask what the preferred alternative
is, to concatenate multiple files together - in one step - if "concat"
is removed?

The documentation currently only lists streamable formats (like MPEG),
and the usage of pipes [1].



Thanks,
Pb


== References:
[1] https://libav.org/faq.html#How-can-I-join-video-files_003f



___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 26/01/16 01:02, Andreas Cadhalpun wrote:
> On 22.01.2016 00:34, Luca Barbato wrote:
>> Let's try to make sure we are talking about the same problem here.
>>
>> by using hls you might craft a playlist containing a concat of a
>> playlist w/out a final new line.
>>
>> So you would send the initial part of the file together with the url.
>>
>> This is an information leak.
> 
> Yes.
> 
>> It is moderate since the url has a maximum size that is sort of small
>> and libav's concat does not have a mean to send a file in chunks.
>>
>> So you cannot send /etc/shadow as whole anyway.
>>
>> The ways to fix the specific problem problem:
>>
>> - provide a blacklist/whitelist option in hls (from me, first
>> solution)-> Anton disliked it since it is too specific,
> 
> However, the hls demuxer should somehow validate the URLs it's reading.
> Your suggestion would be one way to do that, others are possible as well.

As long the solution is not brittle I guess there will be agreement on
it, in the mean time I'd disable concat.

>> courmish pointed out you can feed such line over any playlist some
>> application using avformat supports.
> 
> Likewise such applications should check URLs before passing them
> to libavformat. Though, in practice, there will always be some that don't.
> 
>> - have a pluggable avio-wide infrastructure to policy protocols and
>> paths (from Anton) -> It isn't simple to complete it.
> 
> It's certainly a nice feature for applications to be able to implement
> their IO policy, but this still leaves the question of what should happen
> by default open.
> So this patch set is rather orthogonal to finding a solution for
> preventing these kind of information leaks.

I'd say that is a prerequisite =)

>> - drop concat (agreed by me, Anton and Rémi) -> It is radical, but the
>> feature itself is quite fringe and I would rather drop it.
> 
> That's more like admitting defeat. It's also no real solution as other
> protocols possibly added in the future could be used for similar exploits.

Hopefully not.

>> avconv itself may stay compatible with scripts by implementing it in
>> program itself
> 
> That sounds like a giant hack.

Not that much, it is sort of a little hack to begin with and its main
use-case was to take files split in chunks and not have to cat them again.

>> or providing a better interface for it while at it.
> 
> Isn't the concat demuxer such a better interface?

It is something different (the concat protocol is _that_ kind of fringe).

I meant that having | in the command line is annoying.

>> And what do you see as "the underlying problem"?
> 
> I think that is libavformat mixing local and remote data. If it wouldn't
> to do that by default, such information leaks shouldn't be possible.

it is inherently what hls does, you cannot even restrict to same-source
and not break normal deploys of it =/

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

Quoting Andreas Cadhalpun (2016-01-26 01:02:04)
> On 22.01.2016 13:37, Anton Khirnov wrote:
> > Just so it's clear what we're talking about, what is "properly" for you?
> 
> That would be a more or less general mechanism, which would have prevented
> the information leak in the hls demuxer by default. It should also prevent
> any such possible problems in other demuxers.
> This could be done by implementing a protocol_whitelist with sensible
> defaults.

That was my first idea as well. Turns out, it's rather nontrivial to
implement, due to the fact that protocols allow nesting (e.g. http over
tcp). It would also require the calling programs to have quite a lot of
knowledge about the libav protocol layer internals.

I think Rémi also had other objections, on which he'll hopefully
elaborate himself.

> 
> > And what do you see as "the underlying problem"?
> 
> I think that is libavformat mixing local and remote data. If it wouldn't
> to do that by default, such information leaks shouldn't be possible.
> 

Opening a local playlist that references remote streams is quite common,
so making this behaviour forbidden by default is likely to break many
applications.

So while I fully agree that the concat protocol is not the root problem,
it is the source of its most significat symptom and removing it will
prevent the worst leaks. Meanwhile, we will have time to introduce the
infrastructure to fix this properly (which I think I already started,
but finishing it is, as said above, nontrivial).
And I'd really rather not design new APIs at gunpoint from pressing
security bugs.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Rémi Denis-Courmont]

   Hello,

On Thursday 21 January 2016 23:03:25 Andreas Cadhalpun wrote:
> Why not fix the issue properly instead of removing useful functionality?

By its very essence, the concat protocol allows for injection attacks with 
untrusted URLs (the same super-class of vulnerabilities as XSS and SQL 
injection).

Either you remove that functionality, or you ensure that all URls ever passed 
to libavformat trusted. Best of luck with the latter option.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 26.01.2016 09:52, Luca Barbato wrote:
> On 26/01/16 01:02, Andreas Cadhalpun wrote:
>> On 22.01.2016 00:34, Luca Barbato wrote:
>>> The ways to fix the specific problem problem:
>>>
>>> - provide a blacklist/whitelist option in hls (from me, first
>>> solution)-> Anton disliked it since it is too specific,
>>
>> However, the hls demuxer should somehow validate the URLs it's reading.
>> Your suggestion would be one way to do that, others are possible as well.
> 
> As long the solution is not brittle I guess there will be agreement on
> it, in the mean time I'd disable concat.

I think that at the very least the hls demuxer should always reject protocols
internal to libavformat, like concat, as those simply do not belong into a hls
playlist.

>>> - have a pluggable avio-wide infrastructure to policy protocols and
>>> paths (from Anton) -> It isn't simple to complete it.
>>
>> It's certainly a nice feature for applications to be able to implement
>> their IO policy, but this still leaves the question of what should happen
>> by default open.
>> So this patch set is rather orthogonal to finding a solution for
>> preventing these kind of information leaks.
> 
> I'd say that is a prerequisite =)

Not necessarily. This could also be implemented in the URLContext layer,
accessible via AVOptions.

>>> - drop concat (agreed by me, Anton and Rémi) -> It is radical, but the
>>> feature itself is quite fringe and I would rather drop it.
>>
>> That's more like admitting defeat. It's also no real solution as other
>> protocols possibly added in the future could be used for similar exploits.
> 
> Hopefully not.

Both the hls/applehttp demuxer and the concat protocol existed since 2010,
but only now the problem was realized. That does not make me confident that
problems with future protocols would be realized early on.

>>> avconv itself may stay compatible with scripts by implementing it in
>>> program itself
>>
>> That sounds like a giant hack.
> 
> Not that much, it is sort of a little hack to begin with and its main
> use-case was to take files split in chunks and not have to cat them again.

It still doesn't sound appealing to handle the concat protocol separately,
while all other protocols are handled by libavformat.

>>> or providing a better interface for it while at it.
>>
>> Isn't the concat demuxer such a better interface?
> 
> It is something different (the concat protocol is _that_ kind of fringe).
> 
> I meant that having | in the command line is annoying.

OK, but changing the semantics would also break backwards compatibility.
So if you want to replace the concat protocol with something better,
there should be a reasonable transition period so that people could
adapt to the change over time.

>>> And what do you see as "the underlying problem"?
>>
>> I think that is libavformat mixing local and remote data. If it wouldn't
>> to do that by default, such information leaks shouldn't be possible.
> 
> it is inherently what hls does,

Which is inherently insecure as it is prone to cause information leaks.

> you cannot even restrict to same-source and not break normal deploys of it =/

Maybe we should break those by default to make people aware of the potential
problems. If they want to do that anyway, they can set the appropriate options.

Best regards,
Andreas

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 26.01.2016 19:42, Anton Khirnov wrote:
> Quoting Andreas Cadhalpun (2016-01-26 01:02:04)
>> On 22.01.2016 13:37, Anton Khirnov wrote:
>>> Just so it's clear what we're talking about, what is "properly" for you?
>>
>> That would be a more or less general mechanism, which would have prevented
>> the information leak in the hls demuxer by default. It should also prevent
>> any such possible problems in other demuxers.
>> This could be done by implementing a protocol_whitelist with sensible
>> defaults.
> 
> That was my first idea as well. Turns out, it's rather nontrivial to
> implement, due to the fact that protocols allow nesting (e.g. http over
> tcp).

Could you explain in more detail what problem that would cause?
The whitelist should simply be passed from http to tcp in that case.

> It would also require the calling programs to have quite a lot of
> knowledge about the libav protocol layer internals.

Most programs should be able to work with the default.

>>> And what do you see as "the underlying problem"?
>>
>> I think that is libavformat mixing local and remote data. If it wouldn't
>> to do that by default, such information leaks shouldn't be possible.
>>
> 
> Opening a local playlist that references remote streams is quite common,

Though it is a lot less common than simply opening a local file.

> so making this behaviour forbidden by default is likely to break many
> applications.

I think that applications wanting this behavior should explicitly enable
it. To ease the transition period, one could set the protocol_whitelist
to something sensible for this case, e.g. 'file,http,https,tcp', when the
filename has the extension of a playlist.

> So while I fully agree that the concat protocol is not the root problem,
> it is the source of its most significat symptom and removing it will
> prevent the worst leaks.

That could also be done by disabling it by default, i.e. unless an option
to enable it is given. That way people using it could at least continue
to do so after setting said option.

> Meanwhile, we will have time to introduce the
> infrastructure to fix this properly (which I think I already started,
> but finishing it is, as said above, nontrivial).
> And I'd really rather not design new APIs at gunpoint from pressing
> security bugs.

Then just add quick hacks preventing the worst problems.
I'd rather not have functionality hastily removed due to security
concerns, that could be easily worked around until a proper solution
is found.

Best regards,
Andreas

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

Hi Rémi,

On 26.01.2016 19:49, Rémi Denis-Courmont wrote:
> On Thursday 21 January 2016 23:03:25 Andreas Cadhalpun wrote:
>> Why not fix the issue properly instead of removing useful functionality?
> 
> By its very essence, the concat protocol allows for injection attacks with 
> untrusted URLs (the same super-class of vulnerabilities as XSS and SQL 
> injection).

That's not necessarily the case, if it is reasonably restricted by default.

> Either you remove that functionality, or you ensure that all URls ever passed 
> to libavformat trusted. Best of luck with the latter option.

One can also apply restrictions for URLs passed to libavformat that by default
prevent information leaks.

Best regards,
Andreas
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 27/01/16 01:15, Andreas Cadhalpun wrote:
> I think that at the very least the hls demuxer should always reject protocols
> internal to libavformat, like concat, as those simply do not belong into a hls
> playlist.

There is a patch from yours truly that does that, you are welcome to +1
it =)

concat is not used for anything important (if is used) since it is a
pain to use to begin with so I have no problem in agreeing with what the
others proposed.

I gave already the option you seem to like, you did not +1 it so there
isn't much I can say.

In Gentoo I already disabled concat btw.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 27.01.2016 01:21, Luca Barbato wrote:
> On 27/01/16 01:15, Andreas Cadhalpun wrote:
>> I think that at the very least the hls demuxer should always reject protocols
>> internal to libavformat, like concat, as those simply do not belong into a 
>> hls
>> playlist.
> 
> There is a patch from yours truly that does that, you are welcome to +1
> it =)

I've commented on it now.

> concat is not used for anything important (if is used) since it is a
> pain to use to begin with so I have no problem in agreeing with what the
> others proposed.
> 
> I gave already the option you seem to like, you did not +1 it so there
> isn't much I can say.

I'm mainly interested in fixing the general problem.

Best regards,
Andreas
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 22.01.2016 00:34, Luca Barbato wrote:
> Let's try to make sure we are talking about the same problem here.
> 
> by using hls you might craft a playlist containing a concat of a
> playlist w/out a final new line.
> 
> So you would send the initial part of the file together with the url.
> 
> This is an information leak.

Yes.

> It is moderate since the url has a maximum size that is sort of small
> and libav's concat does not have a mean to send a file in chunks.
> 
> So you cannot send /etc/shadow as whole anyway.
> 
> The ways to fix the specific problem problem:
> 
> - provide a blacklist/whitelist option in hls (from me, first
> solution)-> Anton disliked it since it is too specific,

However, the hls demuxer should somehow validate the URLs it's reading.
Your suggestion would be one way to do that, others are possible as well.

> courmish pointed out you can feed such line over any playlist some
> application using avformat supports.

Likewise such applications should check URLs before passing them
to libavformat. Though, in practice, there will always be some that don't.

> - have a pluggable avio-wide infrastructure to policy protocols and
> paths (from Anton) -> It isn't simple to complete it.

It's certainly a nice feature for applications to be able to implement
their IO policy, but this still leaves the question of what should happen
by default open.
So this patch set is rather orthogonal to finding a solution for
preventing these kind of information leaks.

> - drop concat (agreed by me, Anton and Rémi) -> It is radical, but the
> feature itself is quite fringe and I would rather drop it.

That's more like admitting defeat. It's also no real solution as other
protocols possibly added in the future could be used for similar exploits.

> avconv itself may stay compatible with scripts by implementing it in
> program itself

That sounds like a giant hack.

> or providing a better interface for it while at it.

Isn't the concat demuxer such a better interface?

On 22.01.2016 13:37, Anton Khirnov wrote:
> Just so it's clear what we're talking about, what is "properly" for you?

That would be a more or less general mechanism, which would have prevented
the information leak in the hls demuxer by default. It should also prevent
any such possible problems in other demuxers.
This could be done by implementing a protocol_whitelist with sensible
defaults.

> And what do you see as "the underlying problem"?

I think that is libavformat mixing local and remote data. If it wouldn't
to do that by default, such information leaks shouldn't be possible.

Best regards,
Andreas

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

Quoting Andreas Cadhalpun (2016-01-21 23:03:25)
> On 20.01.2016 13:42, Anton Khirnov wrote:
> > It is of very limited usefulness and is a source of important security
> > problems.
> > 
> > Bug-Id: CVE-2016-1897
> > Bug-Id: CVE-2016-1898
> > ---
> >  Changelog|   1 +
> >  doc/protocols.texi   |  26 ---
> >  libavformat/Makefile |   1 -
> >  libavformat/allformats.c |   1 -
> >  libavformat/concat.c | 191 
> > ---
> >  5 files changed, 1 insertion(+), 219 deletions(-)
> >  delete mode 100644 libavformat/concat.c
> 
> Why not fix the issue properly instead of removing useful functionality?
> 
> Removing this protocol doesn't fix the underlying problem, so another
> protocol, which might be added at some point in the future, could
> expose the same vulnerabilities.

Just so it's clear what we're talking about, what is "properly" for you?
And what do you see as "the underlying problem"?

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 21/01/16 23:35, Andreas Cadhalpun wrote:
> On 21.01.2016 23:21, Luca Barbato wrote:
>> On 21/01/16 23:03, Andreas Cadhalpun wrote:
>>> Why not fix the issue properly instead of removing useful functionality?
>>
>> It is not exactly useful (since it is quite unwieldy)
> 
> But I'm sure it's used in quite some scripts.
> 
>> and the initial
>> infrastructure to policy path and protocols is already in the ml.
> 
> I don't see how that would fix this problem.

Let's try to make sure we are talking about the same problem here.

by using hls you might craft a playlist containing a concat of a
playlist w/out a final new line.

So you would send the initial part of the file together with the url.

This is an information leak.

It is moderate since the url has a maximum size that is sort of small
and libav's concat does not have a mean to send a file in chunks.

So you cannot send /etc/shadow as whole anyway.

The ways to fix the specific problem problem:

- provide a blacklist/whitelist option in hls (from me, first
solution)-> Anton disliked it since it is too specific, courmish pointed
out you can feed such line over any playlist some application using
avformat supports.

- have a pluggable avio-wide infrastructure to policy protocols and
paths (from Anton) -> It isn't simple to complete it.

- drop concat (agreed by me, Anton and Rémi) -> It is radical, but the
feature itself is quite fringe and I would rather drop it.

avconv itself may stay compatible with scripts by implementing it in
program itself or providing a better interface for it while at it.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 21/01/16 23:03, Andreas Cadhalpun wrote:
> Why not fix the issue properly instead of removing useful functionality?

It is not exactly useful (since it is quite unwieldy) and the initial
infrastructure to policy path and protocols is already in the ml.

So the two are sort of orthogonal.

Concat can be misused doesn't matter what by it's nature as courmish
pointed out on IRC.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 20.01.2016 13:42, Anton Khirnov wrote:
> It is of very limited usefulness and is a source of important security
> problems.
> 
> Bug-Id: CVE-2016-1897
> Bug-Id: CVE-2016-1898
> ---
>  Changelog|   1 +
>  doc/protocols.texi   |  26 ---
>  libavformat/Makefile |   1 -
>  libavformat/allformats.c |   1 -
>  libavformat/concat.c | 191 
> ---
>  5 files changed, 1 insertion(+), 219 deletions(-)
>  delete mode 100644 libavformat/concat.c

Why not fix the issue properly instead of removing useful functionality?

Removing this protocol doesn't fix the underlying problem, so another
protocol, which might be added at some point in the future, could
expose the same vulnerabilities.

Best regards,
Andreas
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Andreas Cadhalpun]

On 21.01.2016 23:21, Luca Barbato wrote:
> On 21/01/16 23:03, Andreas Cadhalpun wrote:
>> Why not fix the issue properly instead of removing useful functionality?
> 
> It is not exactly useful (since it is quite unwieldy)

But I'm sure it's used in quite some scripts.

> and the initial
> infrastructure to policy path and protocols is already in the ml.

I don't see how that would fix this problem.

> So the two are sort of orthogonal.

Then why do you bring this up here?

> Concat can be misused doesn't matter what by it's nature as courmish
> pointed out on IRC.

I don't think that's true.
How did he come to that conclusion?

Best regards,
Andreas

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Vittorio Giovara]

On Wed, Jan 20, 2016 at 7:42 AM, Anton Khirnov  wrote:
> It is of very limited usefulness and is a source of important security
> problems.
>
> Bug-Id: CVE-2016-1897
> Bug-Id: CVE-2016-1898
> ---

I'm ok with removing this, however shouldn't this be followed by a
version bump of some sort?
I'm not sure whether this is 'only' an API break or if it's inherently
an API+ABI break.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Rémi Denis-Courmont]

On Wednesday 20 January 2016 13:42:43 Anton Khirnov wrote:
> It is of very limited usefulness and is a source of important security
> problems.

I suggested it so obviously fine with me.

If someone really has a use for it, I´d suggest configuring it via out-of-band 
options.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Luca Barbato]

On 20/01/16 18:41, Vittorio Giovara wrote:
> On Wed, Jan 20, 2016 at 7:42 AM, Anton Khirnov  wrote:
>> It is of very limited usefulness and is a source of important security
>> problems.
>>
>> Bug-Id: CVE-2016-1897
>> Bug-Id: CVE-2016-1898
>> ---
> 
> I'm ok with removing this, however shouldn't this be followed by a
> version bump of some sort?
> I'm not sure whether this is 'only' an API break or if it's inherently
> an API+ABI break.
> 

This fringe protocol isn't really used much that I know.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Vittorio Giovara]

On Wed, Jan 20, 2016 at 3:23 PM, Anton Khirnov  wrote:
> Quoting Vittorio Giovara (2016-01-20 18:41:57)
>> On Wed, Jan 20, 2016 at 7:42 AM, Anton Khirnov  wrote:
>> > It is of very limited usefulness and is a source of important security
>> > problems.
>> >
>> > Bug-Id: CVE-2016-1897
>> > Bug-Id: CVE-2016-1898
>> > ---
>>
>> I'm ok with removing this, however shouldn't this be followed by a
>> version bump of some sort?
>> I'm not sure whether this is 'only' an API break or if it's inherently
>> an API+ABI break.
>
> It's neither, adding or removing and muxers, demuxers, protocols,
> codecs, filters, whatever has, in itself, no effect on API or ABI.

well not a break per se, but if a user tries to use one of the removed
features it will just fail to do so, breaking their code. Anyway this
particular case is fringe enough that it probably does not matter.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

Quoting Vittorio Giovara (2016-01-20 18:41:57)
> On Wed, Jan 20, 2016 at 7:42 AM, Anton Khirnov  wrote:
> > It is of very limited usefulness and is a source of important security
> > problems.
> >
> > Bug-Id: CVE-2016-1897
> > Bug-Id: CVE-2016-1898
> > ---
> 
> I'm ok with removing this, however shouldn't this be followed by a
> version bump of some sort?
> I'm not sure whether this is 'only' an API break or if it's inherently
> an API+ABI break.

It's neither, adding or removing and muxers, demuxers, protocols,
codecs, filters, whatever has, in itself, no effect on API or ABI.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] lavf: remove the concat protocol

[Anton Khirnov]

It is of very limited usefulness and is a source of important security
problems.

Bug-Id: CVE-2016-1897
Bug-Id: CVE-2016-1898
---
 Changelog|   1 +
 doc/protocols.texi   |  26 ---
 libavformat/Makefile |   1 -
 libavformat/allformats.c |   1 -
 libavformat/concat.c | 191 ---
 5 files changed, 1 insertion(+), 219 deletions(-)
 delete mode 100644 libavformat/concat.c

diff --git a/Changelog b/Changelog
index 92c694bc..6d55284 100644
--- a/Changelog
+++ b/Changelog
@@ -51,6 +51,7 @@ version :
 - support Apple AVFoundation video capture
 - G.723.1 muxer and encoder
 - compressed SWF
+- dropped the concat protocol, because of security issues
 
 
 version 11:
diff --git a/doc/protocols.texi b/doc/protocols.texi
index f30567d..4c59dff 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -19,32 +19,6 @@ supported protocols.
 
 A description of the currently available protocols follows.
 
-@section concat
-
-Physical concatenation protocol.
-
-Allow to read and seek from many resource in sequence as if they were
-a unique resource.
-
-A URL accepted by this protocol has the syntax:
-@example
-concat:@var{URL1}|@var{URL2}|...|@var{URLN}
-@end example
-
-where @var{URL1}, @var{URL2}, ..., @var{URLN} are the urls of the
-resource to be concatenated, each one possibly specifying a distinct
-protocol.
-
-For example to read a sequence of files @file{split1.mpeg},
-@file{split2.mpeg}, @file{split3.mpeg} with @command{avplay} use the
-command:
-@example
-avplay concat:split1.mpeg\|split2.mpeg\|split3.mpeg
-@end example
-
-Note that you may need to escape the character "|" which is special for
-many shells.
-
 @section file
 
 File access protocol.
diff --git a/libavformat/Makefile b/libavformat/Makefile
index c5d1bfa..7e2c506 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -377,7 +377,6 @@ OBJS-$(CONFIG_LIBRTMP)   += librtmp.o
 
 # protocols I/O
 OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
-OBJS-$(CONFIG_CONCAT_PROTOCOL)   += concat.o
 OBJS-$(CONFIG_CRYPTO_PROTOCOL)   += crypto.o
 OBJS-$(CONFIG_FFRTMPCRYPT_PROTOCOL)  += rtmpcrypt.o rtmpdh.o
 OBJS-$(CONFIG_FFRTMPHTTP_PROTOCOL)   += rtmphttp.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index a514c63..03d610d 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -266,7 +266,6 @@ void av_register_all(void)
 REGISTER_MUXDEMUX(YUV4MPEGPIPE, yuv4mpegpipe);
 
 /* protocols */
-REGISTER_PROTOCOL(CONCAT,   concat);
 REGISTER_PROTOCOL(CRYPTO,   crypto);
 REGISTER_PROTOCOL(FFRTMPCRYPT,  ffrtmpcrypt);
 REGISTER_PROTOCOL(FFRTMPHTTP,   ffrtmphttp);
diff --git a/libavformat/concat.c b/libavformat/concat.c
deleted file mode 100644
index 2fb3ba9..000
--- a/libavformat/concat.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/*
- * Concat URL protocol
- * Copyright (c) 2006 Steve Lhomme
- * Copyright (c) 2007 Wolfram Gloger
- * Copyright (c) 2010 Michele Orrù
- *
- * This file is part of Libav.
- *
- * Libav is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * Libav is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with Libav; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include "libavutil/avstring.h"
-#include "libavutil/mem.h"
-
-#include "avformat.h"
-#include "url.h"
-
-#define AV_CAT_SEPARATOR "|"
-
-struct concat_nodes {
-URLContext *uc;///< node's URLContext
-int64_t size;  ///< url filesize
-};
-
-struct concat_data {
-struct concat_nodes *nodes;///< list of nodes to concat
-size_t   length;   ///< number of cat'ed nodes
-size_t   current;  ///< index of currently read node
-};
-
-static av_cold int concat_close(URLContext *h)
-{
-int err = 0;
-size_t i;
-struct concat_data  *data  = h->priv_data;
-struct concat_nodes *nodes = data->nodes;
-
-for (i = 0; i != data->length; i++)
-err |= ffurl_close(nodes[i].uc);
-
-av_freep(>nodes);
-
-return err < 0 ? -1 : 0;
-}
-
-static av_cold int concat_open(URLContext *h, const char *uri, int flags)
-{
-char *node_uri = NULL;
-int err = 0;
-int64_t size;
-size_t len, i;
-URLContext *uc;
-struct concat_data  *data = h->priv_data;
-struct concat_nodes *nodes;
-
-av_strstart(uri, "concat:", );
-
-for (i = 0, len = 1;