[FFmpeg-devel] [PATCH] avformat/file: add fd protocol

[Rémi Denis-Courmont]

Le maanantaina 12. joulukuuta 2022, 19.23.52 EET Zhao Zhili a écrit :
> On Mon, 2022-12-12 at 18:31 +0200, Rémi Denis-Courmont wrote:
> > Le maanantaina 5. joulukuuta 2022, 4.51.34 EET zhilizhao(赵志立) a écrit
> > 
> > > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack at foxmail.com>
> > > > wrote:
> > > > 
> > > > From: Zhao Zhili <zhilizhao at tencent.com>
> > > > 
> > > > Unlike the pipe protocol, fd protocol has seek support if it
> > > > corresponding to a regular file.
> > > > ---
> > > > Sometimes it's the only way to access files via file descriptor,
> > > > e.g.,
> > > > requesting a shared file on Android:
> > > > https://developer.android.com/training/secure-file-sharing/request-fil
> > > > e
> > > > 
> > > > doc/protocols.texi      | 24 +++++++++++++++++++
> > > > libavformat/Makefile    |  1 +
> > > > libavformat/file.c      | 51
> > > > +++++++++++++++++++++++++++++++++++++++++
> > > > libavformat/protocols.c |  1 +
> > > > libavformat/version.h   |  4 ++--
> > > > 5 files changed, 79 insertions(+), 2 deletions(-)
> > > 
> > > Ping for review.
> > 
> > VLC does this (with a slightly different syntax, i.e. fd://$NUM) and
> > in
> > hindsight, I think that it was a big mistake.
> > 
> > It should not be possible to refer to an opaque handle from within
> > the URL.
> > This leads to all sorts of mischief, bordering on security issue, in
> > the
> > common case that the URL string is untrusted.
> 
> Could you elaborate on the security issue?

Just guess the FD of some critical process resource (.e.g D-Bus, X11 or 
Wayland connection), , and you can easily cause a DoS. It gets potentially 
worse if you can guess the FD that is used to carry sensitive data, such as 
another clients connection if FFmpeg runs inside some kind of network service.

> > To support this use case, IMO, the file descriptor should be passed
> > explicitly
> > via a trusted channel, *not* the URL.
> 
> Does an explicit option works here?
> 
>  static const AVOption pipe_options[] = {
>      { "blocksize", "set I/O operation maximum block size",
> offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX },
> 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
> +    { "fd", "set file descriptor", offsetof(FileContext, fd),
> AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM
> },
>      { NULL }
>  };

That seems more reasonable, so I cannot comment on the specifics of AVOption 
usage.


-- 
Реми Дёни-Курмон
http://www.remlab.net/