Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -safe to replace the user name and password in the protocol address

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> On Dec 17, 2022, at 15:36, Wujian(Chin) <wujian2@huawei.com> wrote:
> 
> The Protocol address may contain the user name and password. The ps -ef command may expose the plaintext.
> The -safe parameter option is added to replace the user name and password in the command line with the asterisk (*).

The patch reduced the risk to a low level, but I don’t think it
fixed the security issue totally. It’s still there with a small
time window. The usecase itself is unsafe.

There is an -safe option in concat demuxer, please make sure there
is no conflict.

concat demuxer AVOptions:
  -safe              <boolean>    .D......... enable safe mode (default true)

> 
> Signed-off-by: wujian_nanjing <wujian2@huawei.com>
> ---
> doc/ffmpeg.texi    |  7 +++++++
> doc/ffplay.texi    |  8 ++++++++
> doc/ffprobe.texi   |  7 +++++++
> fftools/cmdutils.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
> fftools/cmdutils.h | 15 +++++++++++++++
> fftools/ffmpeg.c   | 16 +++++++++++++---
> fftools/ffplay.c   | 15 +++++++++++++--
> fftools/ffprobe.c  | 18 ++++++++++++++----
> 8 files changed, 120 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 0367930..e905542 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -50,6 +50,13 @@ output files. Also do not mix options which belong to different files. All
> options apply ONLY to the next input or output file and are reset between files.
> 
> @itemize
> +@item -safe
> +The Protocol address may contain the user name and password. The ps -ef command may expose the plaintext.
> +The -safe parameter option is added to replace the user name and password in the command line with the asterisk (*).
> +@example
> +ffmpeg -safe -i rtsp://username@password.xxxx.com
> +@end example
> +
> @item
> To set the video bitrate of the output file to 64 kbit/s:
> @example
> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
> index 5dd860b..f46ca91 100644
> --- a/doc/ffplay.texi
> +++ b/doc/ffplay.texi
> @@ -122,6 +122,14 @@ Read @var{input_url}.
> 
> @section Advanced options
> @table @option
> +
> +@item -safe
> +The Protocol address may contain the user name and password. The ps -ef command may expose the plaintext.
> +The -safe parameter option is added to replace the user name and password in the command line with the asterisk (*).
> +@example
> +ffplay -safe -i rtsp://username@password.xxxx.com
> +@end example
> +
> @item -stats
> Print several playback statistics, in particular show the stream
> duration, the codec parameters, the current position in the stream and
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index 4dc9f57..92b13cf 100644
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
> @@ -89,6 +89,13 @@ Set the output printing format.
> @var{writer_name} specifies the name of the writer, and
> @var{writer_options} specifies the options to be passed to the writer.
> 
> +@item -safe
> +The Protocol address may contain the user name and password. The ps -ef command may expose the plaintext.
> +The -safe parameter option is added to replace the user name and password in the command line with the asterisk (*).
> +@example
> +ffprobe -safe -i rtsp://username@password.xxxx.com
> +@end example
> +
> For example for printing the output in JSON format, specify:
> @example
> -print_format json
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index a1de621..22407f8 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -61,6 +61,40 @@ AVDictionary *format_opts, *codec_opts;
> 
> int hide_banner = 0;
> 
> +void param_masking(int argc, char **argv) {
> +    int i, j;
> +    for (i = 1; i < argc; i++) {
> +        char *match = strstr(argv[i], "://");
> +        if (match) {
> +            int total = strlen(argv[i]);
> +            for (j = 0; j < total; j++) {
> +                argv[i][j] = '*';
> +            }
> +        }
> +    }
> +}
> +
> +char **copy_argv(int argc, char **argv) {
> +    char **argv2;
> +    argv2 = av_mallocz(argc * sizeof(char *));
> +    if (!argv2)
> +        exit_program(1);
> +
> +    for (int i = 0; i < argc; i++) {
> +        int length = strlen(argv[i]) + 1;
> +        argv2[i] = av_mallocz(length * sizeof(char *));
> +        if (!argv2[i])
> +            exit_program(1);
> +        memcpy(argv2[i], argv[i], length - 1);
> +    }
> +    return argv2;
> +}
> +
> +void free_pp(int argc, char **argv) {
> +    for (int i = 0; i < argc; i++)
> +        av_free(argv[i]);
> +    av_free(argv);
> +}
> void uninit_opts(void)
> {
>     av_dict_free(&swr_opts);
> @@ -215,13 +249,13 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
>     if (win32_argv_utf8) {
>         *argc_ptr = win32_argc;
>         *argv_ptr = win32_argv_utf8;
> -        return;
> +        goto end;
>     }
> 
>     win32_argc = 0;
>     argv_w = CommandLineToArgvW(GetCommandLineW(), &win32_argc);
>     if (win32_argc <= 0 || !argv_w)
> -        return;
> +        goto end;
> 
>     /* determine the UTF-8 buffer size (including NULL-termination symbols) */
>     for (i = 0; i < win32_argc; i++)
> @@ -232,7 +266,7 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
>     argstr_flat     = (char *)win32_argv_utf8 + sizeof(char *) * (win32_argc + 1);
>     if (!win32_argv_utf8) {
>         LocalFree(argv_w);
> -        return;
> +        goto end;
>     }
> 
>     for (i = 0; i < win32_argc; i++) {
> @@ -243,9 +277,14 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
>     }
>     win32_argv_utf8[i] = NULL;
>     LocalFree(argv_w);
> -
>     *argc_ptr = win32_argc;
>     *argv_ptr = win32_argv_utf8;
> +end:
> +    if (*argc_ptr > 1 && !strcmp((*argv_ptr)[1], "-safe")) {
> +        (*argv_ptr)[1] = (*argv_ptr)[0];
> +        (*argc_ptr)--;
> +        (*argv_ptr)++;
> +    }
> }
> #else
> static inline void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 4496221..ce4c1db 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -50,6 +50,21 @@ extern AVDictionary *format_opts, *codec_opts;
> extern int hide_banner;
> 
> /**
> + * Using to masking sensitive info.
> + */
> +void param_masking(int argc, char **argv);
> +
> +/**
> + * Using to copy ori argv.
> + */
> +char **copy_argv(int argc, char **argv);
> +
> +/**
> + * Free **
> + */
> +void free_pp(int argc, char **argv);
> +
> +/**
>  * Register a program-specific cleanup routine.
>  */
> void register_exit(void (*cb)(int ret));
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 881d6f0..f77e850 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -3865,9 +3865,9 @@ static int64_t getmaxrss(void)
> 
> int main(int argc, char **argv)
> {
> -    int ret;
> +    int ret, safeFlag;
>     BenchmarkTimeStamps ti;
> -
> +    char **argv2;
>     init_dynload();
> 
>     register_exit(ffmpeg_cleanup);
> @@ -3877,15 +3877,25 @@ int main(int argc, char **argv)
>     av_log_set_flags(AV_LOG_SKIP_REPEATED);
>     parse_loglevel(argc, argv, options);
> 
> +    safeFlag = 0;
> +    if (argc > 1 && !strcmp(argv[1], "-safe")) {
> +        argv[1] = argv[0];
> +        safeFlag = 1;
> +        argc--;
> +        argv++;
> +    }
> #if CONFIG_AVDEVICE
>     avdevice_register_all();
> #endif
>     avformat_network_init();
> 
>     show_banner(argc, argv, options);
> +    argv2 = copy_argv(argc, argv);
> +    if (safeFlag)
> +        param_masking(argc, argv);
> 
>     /* parse options and open all input/output files */
> -    ret = ffmpeg_parse_options(argc, argv);
> +    ret = ffmpeg_parse_options(argc, argv2);
>     if (ret < 0)
>         exit_program(1);
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index fc7e1c2..f9e6c91 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -3663,10 +3663,18 @@ void show_help_default(const char *opt, const char *arg)
> /* Called from the main */
> int main(int argc, char **argv)
> {
> -    int flags;
> +    int flags, safeFlag;
> +    char **argv2;
>     VideoState *is;
> 
>     init_dynload();
> +    safeFlag = 0;
> +    if (argc > 1 && !strcmp(argv[1], "-safe")) {
> +        argv[1] = argv[0];
> +        safeFlag = 1;
> +        argc--;
> +        argv++;
> +    }
> 
>     av_log_set_flags(AV_LOG_SKIP_REPEATED);
>     parse_loglevel(argc, argv, options);
> @@ -3682,7 +3690,10 @@ int main(int argc, char **argv)
> 
>     show_banner(argc, argv, options);
> 
> -    parse_options(NULL, argc, argv, options, opt_input_file);
> +    argv2 = copy_argv(argc, argv);
> +    parse_options(NULL, argc, argv2, options, opt_input_file);
> +    if (safeFlag)
> +        param_masking(argc, argv);
> 
>     if (!input_filename) {
>         show_usage();
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index d2f126d..8d4d1e9 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -4035,9 +4035,16 @@ int main(int argc, char **argv)
>     WriterContext *wctx;
>     char *buf;
>     char *w_name = NULL, *w_args = NULL;
> -    int ret, input_ret, i;
> -
> +    int ret, input_ret, i, safeFlag;
> +    char **argv2;
>     init_dynload();
> +    safeFlag = 0;
> +    if (argc > 1 && !strcmp(argv[1], "-safe")) {
> +        argv[1] = argv[0];
> +        safeFlag = 1;
> +        argc--;
> +        argv++;
> +    }
> 
> #if HAVE_THREADS
>     ret = pthread_mutex_init(&log_mutex, NULL);
> @@ -4056,8 +4063,10 @@ int main(int argc, char **argv)
> #endif
> 
>     show_banner(argc, argv, options);
> -    parse_options(NULL, argc, argv, options, opt_input_file);
> -
> +    argv2 = copy_argv(argc, argv);
> +    parse_options(NULL, argc, argv2, options, opt_input_file);
> +    if (safeFlag)
> +        param_masking(argc, argv);
>     if (do_show_log)
>         av_log_set_callback(log_callback);
> 
> @@ -4173,6 +4182,7 @@ end:
>     av_freep(&print_format);
>     av_freep(&read_intervals);
>     av_hash_freep(&hash);
> +    free_pp(argc, argv2);
> 
>     uninit_opts();
>     for (i = 0; i < FF_ARRAY_ELEMS(sections); i++)
> -- 
> 2.7.4
> 
> _______________________________________________
> 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".