Wujian(Chin) (12022-12-19):
> I have modified the issues. Please review it again. Thank you.
>
> If the protocol address contains the user name and password, The ps -ef
> command exposes plaintext.
Spurious comma or capital.
> The -mask_url parameter option is added to replace the protocol address in
> the command line with the asterisk (*).
> Because other users can run the ps -ef command to view sensitive information
> such as the user name and password
> in the protocol address, which is insecure.
Please wrap to 60-72 characters.
>
> Signed-off-by: wujian_nanjing <wuji...@huawei.com>
> ---
> doc/ffmpeg.texi | 9 +++++++++
> doc/ffplay.texi | 8 ++++++++
> doc/ffprobe.texi | 9 +++++++++
> fftools/cmdutils.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
> fftools/cmdutils.h | 15 +++++++++++++++
> fftools/ffmpeg.c | 16 +++++++++++++---
> fftools/ffplay.c | 15 +++++++++++++--
> fftools/ffprobe.c | 18 ++++++++++++++----
> 8 files changed, 124 insertions(+), 13 deletions(-)
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 0367930..1f6cb33 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> --- a/doc/ffplay.texi
> +++ b/doc/ffplay.texi
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
The place for common options is doc/fftools-common-opts.texi.
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index a1de621..c35d7e1 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) {
Functions name in ...ing do not seem idiomatic to me.
The style for the brace is off.
> + 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] = '*';
> + }
Masking the whole URL seems too much. Logins and passwords are
introduced by the @ character.
> + }
> + }
> +}
> +
> +char **copy_argv(int argc, char **argv) {
The brace is off here too.
> + char **argv2;
> + argv2 = av_mallocz(argc * sizeof(char *));
sizeof(*argv2)
> + if (!argv2)
> + exit_program(1);
Error message.
> +
> + 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) {
The brace is off too. This function is called only from ffprobe, looks
wrong.
> + 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;
We only use goto for error processing.
> }
>
> 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], "-mask_url")) {
> + (*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..fccbde9 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, maskFlag;
We do not do camelCase.
> 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);
>
> + maskFlag = 0;
> + if (argc > 1 && !strcmp(argv[1], "-mask_url")) {
> + argv[1] = argv[0];
> + maskFlag = 1;
> + argc--;
> + argv++;
> + }
This option is not special nor important enough to warrant a special
treatment like that.
> #if CONFIG_AVDEVICE
> avdevice_register_all();
> #endif
> avformat_network_init();
>
> show_banner(argc, argv, options);
> + argv2 = copy_argv(argc, argv);
> + if (maskFlag)
> + param_masking(argc, argv);
This is duplicated in all three files and unnecessary: have a single
function do the copy and the masking.
>
> /* 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..5d282f1 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, maskFlag;
> + char **argv2;
> VideoState *is;
>
> init_dynload();
> + maskFlag = 0;
> + if (argc > 1 && !strcmp(argv[1], "-mask_url")) {
> + argv[1] = argv[0];
> + maskFlag = 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 (maskFlag)
> + param_masking(argc, argv);
>
> if (!input_filename) {
> show_usage();
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index d2f126d..e69f49f 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, maskFlag;
> + char **argv2;
> init_dynload();
> + maskFlag = 0;
> + if (argc > 1 && !strcmp(argv[1], "-mask_url")) {
> + argv[1] = argv[0];
> + maskFlag = 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 (maskFlag)
> + 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++)
Regards,
--
Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
