| Message ID | 7407e74b181e4e00a7b7104fb63cf56a@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*) | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| yinshiyou/commit_msg_loongarch64 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | fail | Make failed |
| yinshiyou/make_loongarch64 | fail | Make failed |
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 <wujian2@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,
On 19 Dec 2022, at 14:15, Wujian(Chin) wrote: > 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. > 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. > > Signed-off-by: wujian_nanjing <wujian2@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 > @@ -50,6 +50,15 @@ 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 -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +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. > +@example > +ffmpeg -mask_url -i rtsp://username:password-ip:port/stream/test > +@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..b40fe75 100644 > --- a/doc/ffplay.texi > +++ b/doc/ffplay.texi > @@ -120,8 +120,16 @@ sources and sinks). > Read @var{input_url}. > @end table > > +@item -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +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. > +@end table > + > @section Advanced options > @table @option > + > @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..33c0e7d 100644 > --- a/doc/ffprobe.texi > +++ b/doc/ffprobe.texi > @@ -89,6 +89,15 @@ 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 -mask_url -i @var{url} (@emph{output}) > +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. > +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. > +@example > +ffprobe -mask_url -i rtsp://username:password-ip:port/stream/test > +@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..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) { > + 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], "-mask_url")) { > + (*argv_ptr)[1] = (*argv_ptr)[0]; > + (*argc_ptr)--; > + (*argv_ptr)++; > + } IIUC this means the `-mask_url` option has to be the first option passed, which seems a bit of an unfortunate requirement and is not documented at all, as far as I can see. So at least this should be clearly documented to prevent users being confused why the get an unrecognised option error when they do not pass it as the first option. > } > #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; > 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++; > + } > #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); > > /* 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); I am a bit confused how this helps for the issue it tries to solve, as for some amount of time, until this is done, it would expose the full plaintext URL still, no? > 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".
Marvin Scholz (12022-12-19): > IIUC this means the `-mask_url` option has to be the first option passed, > which seems a bit of an unfortunate requirement and is not documented at > all, as far as I can see. So at least this should be clearly documented > to prevent users being confused why the get an unrecognised option error > when they do not pass it as the first option. Indeed. And I see no reason to have this option processed specially like that; it requires at least an explanation. > I am a bit confused how this helps for the issue it tries to solve, as > for some amount of time, until this is done, it would expose the full > plaintext URL still, no? This is unavoidable. Still, having sensitive information visible for a fraction of a second is better than having sensitive information visible for the length of a playback or transcoding process. Regards,
On 2022-12-19 07:00 pm, Nicolas George wrote: > Wujian(Chin) (12022-12-19): >> - return; >> + goto end; > We only use goto for error processing. Not the case. They are used for 'end', 'finish', retries ...etc Regards, Gyan
On 19 Dec 2022, at 14:37, Nicolas George wrote: > Marvin Scholz (12022-12-19): >> IIUC this means the `-mask_url` option has to be the first option passed, >> which seems a bit of an unfortunate requirement and is not documented at >> all, as far as I can see. So at least this should be clearly documented >> to prevent users being confused why the get an unrecognised option error >> when they do not pass it as the first option. > > Indeed. And I see no reason to have this option processed specially like > that; it requires at least an explanation. > >> I am a bit confused how this helps for the issue it tries to solve, as >> for some amount of time, until this is done, it would expose the full >> plaintext URL still, no? > > This is unavoidable. Still, having sensitive information visible for a > fraction of a second is better than having sensitive information visible > for the length of a playback or transcoding process. I agree, but then the docs should probably mention that to not give a false sense of absolute security here. And maybe note that it might be a better option to pass the password via stdin or hide the process from other users to completely avoid leaking the password. > > 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-request@ffmpeg.org with subject "unsubscribe".
Gyan Doshi (12022-12-19):
> Not the case. They are used for 'end', 'finish', retries ...etc
Insufficient vigilance before accepting these changes. But not a reason
to accept more.
Regards,
Marvin Scholz (12022-12-19): > I agree, but then the docs should probably mention that to not give a false > sense of absolute security here. And maybe note that it might Indeed, documentation is necessary. > be a better option to pass the password via stdin or hide the process > from other users to completely avoid leaking the password. Unfortunately, as far as I know, we do not have any mechanism of the kind. That would be useful. Regards,
> On Dec 19, 2022, at 21:40, Marvin Scholz <epirat07@gmail.com> wrote: > > > On 19 Dec 2022, at 14:37, Nicolas George wrote: > >> Marvin Scholz (12022-12-19): >>> IIUC this means the `-mask_url` option has to be the first option passed, >>> which seems a bit of an unfortunate requirement and is not documented at >>> all, as far as I can see. So at least this should be clearly documented >>> to prevent users being confused why the get an unrecognised option error >>> when they do not pass it as the first option. >> >> Indeed. And I see no reason to have this option processed specially like >> that; it requires at least an explanation. >> >>> I am a bit confused how this helps for the issue it tries to solve, as >>> for some amount of time, until this is done, it would expose the full >>> plaintext URL still, no? >> >> This is unavoidable. Still, having sensitive information visible for a >> fraction of a second is better than having sensitive information visible >> for the length of a playback or transcoding process. > > I agree, but then the docs should probably mention that to not give a false > sense of absolute security here. And maybe note that it might > be a better option to pass the password via stdin or hide the process > from other users to completely avoid leaking the password. We have options like ‘-password', ‘-key’, ‘-cryptokey' and so on. I prefer hide the entire argument lists if we accept this solution. I don’t know about system administration, hidepid looks like a neat solution. https://linux-audit.com/linux-system-hardening-adding-hidepid-to-proc/ > >> >> 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-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".
>> @@ -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. I think that it's more concise to use code this way. >> + 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. I think that it would be better to replace the entire url, so that the code implementation is simple. >> + char **argv2; >> + argv2 = av_mallocz(argc * sizeof(char *)); >sizeof(*argv2) > >> + 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. This option needs to replace the URL. It is more appropriate to judge mask_url and copy argv in this place. Otherwise, do you have any other suggestions? Thank you for your issue. Nicolas George -----邮件原件----- 发件人: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] 代表 Nicolas George 发送时间: 2022年12月19日 21:30 收件人: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> 抄送: wangqinghua (I) <wangqinghua9@huawei.com> 主题: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*) 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 <wujian2@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-request@ffmpeg.org with subject "unsubscribe".
>Marvin Scholz (12022-12-19): > I agree, but then the docs should probably mention that to not give a >> false sense of absolute security here. And maybe note that it might >Indeed, documentation is necessary. Is it appropriate to describe this document? Please give some suggestions. Thank you , everyone. fftools-common-opts.texi @item -mask_url -i @var{url} (@emph{output}) If the protocol address contains the user name and password, the ps -ef command exposes plaintext. 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. @example ffmpeg -mask_url -i rtsp://username:password-ip:port/stream/test @end example >> be a better option to pass the password via stdin or hide the process >> from other users to completely avoid leaking the password. >Unfortunately, as far as I know, we do not have any mechanism of the kind. That would be useful. >Regards, -- > Nicolas George
Wujian(Chin) (12022-12-20): > I think that it's more concise to use code this way. Concision is not the goal here, maintainability is. Please do not use gotos. > I think that it would be better to replace the entire url, so that the code implementation is simple. Then replace the whole command line, it is even simpler. Also, this way you miss credentials passed through options. > >> + argv2 = av_mallocz(argc * sizeof(char *)); > > >sizeof(*argv2) Youhoud? > This option needs to replace the URL. It is more appropriate to judge > mask_url and copy argv in this place. Otherwise, do you have any > other suggestions? Use the normal options parsing system. Regards,
On Mon, 19 Dec 2022, "zhilizhao(赵志立)" wrote: > > >> On Dec 19, 2022, at 21:40, Marvin Scholz <epirat07@gmail.com> wrote: >> >> >> On 19 Dec 2022, at 14:37, Nicolas George wrote: >> >>> Marvin Scholz (12022-12-19): >>>> IIUC this means the `-mask_url` option has to be the first option passed, >>>> which seems a bit of an unfortunate requirement and is not documented at >>>> all, as far as I can see. So at least this should be clearly documented >>>> to prevent users being confused why the get an unrecognised option error >>>> when they do not pass it as the first option. >>> >>> Indeed. And I see no reason to have this option processed specially like >>> that; it requires at least an explanation. >>> >>>> I am a bit confused how this helps for the issue it tries to solve, as >>>> for some amount of time, until this is done, it would expose the full >>>> plaintext URL still, no? >>> >>> This is unavoidable. Still, having sensitive information visible for a >>> fraction of a second is better than having sensitive information visible >>> for the length of a playback or transcoding process. >> >> I agree, but then the docs should probably mention that to not give a false >> sense of absolute security here. And maybe note that it might >> be a better option to pass the password via stdin or hide the process >> from other users to completely avoid leaking the password. > > We have options like ‘-password', ‘-key’, ‘-cryptokey' and so on. I prefer > hide the entire argument lists if we accept this solution. I don’t know about > system administration, hidepid looks like a neat solution. > https://linux-audit.com/linux-system-hardening-adding-hidepid-to-proc/ I am not a fan of this masking, because the false sense of security, docs or not. Does wget or curl mask its command line? But I agree, if such "feature" is added, it should remove the whole command line. And the docs should point to real solutions, like hidepid. Regards, Marton
Nicolas George(2022年12月23日 3:27): >Wujian(Chin) (12022-12-20): >> I think that it's more concise to use code this way. >Concision is not the goal here, maintainability is. Please do not use gotos. I found that goto can be used in three cases: 1. Jump directly from multiple cycles; 2. Clear resources when errors occur. 3. Cases where clarity of the procedure can be increased. This code simplicity does not affect the readability and maintainability of the program, and I understand that it belongs to the third case. >> I think that it would be better to replace the entire url, so that the code implementation is simple. >Then replace the whole command line, it is even simpler. Also, this way you miss credentials passed through options. The mask effect is like this：ffmpeg -mask_url -i rtsp://tyyy.com --> ffmpeg -mask_url -i *************** > this way you miss credentials passed through options. I still don't understand. Can you go into more detail? >> >> + argv2 = av_mallocz(argc * sizeof(char *)); >> >> >sizeof(*argv2) >Youhoud? Sorry, I didn't get it. > > This option needs to replace the URL. It is more appropriate to judge > > mask_url and copy argv in this place. Otherwise, do you have any > > other suggestions? > Use the normal options parsing system. I understand that mask_url needs to be placed first, because the FFmpeg process of the ps -ef command is viewed as argv, Therefore, you need to copy argv to argv_copy immediately, mask the URL in the argv, and then argv_copy is a command without mask_url for the following programs. I don't understand the difference between the normal option and the mask_url in use? Looking forward to your reply, thank you. > Regards, -- > Nicolas George _______________________________________________ ffmpeg-user mailing list ffmpeg-user@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-user To unsubscribe, visit link above, or email ffmpeg-user-request@ffmpeg.org with subject "unsubscribe".
Wujian(Chin) (12022-12-24): > This code simplicity does not affect the readability and > maintainability of the program You are not the one who maintain them, that is not your judgement. > The mask effect is like this：ffmpeg -mask_url -i rtsp://tyyy.com --> ffmpeg -mask_url -i *************** I know, it was obvious from the start. > > this way you miss credentials passed through options. > I still don't understand. Can you go into more detail? Look up cryptokey in the documentation for example. > >> >sizeof(*argv2) > Sorry, I didn't get it. This is elementary C and a staple of FFmpeg's coding style. > I understand that mask_url needs to be placed first, And that is an unacceptable misfeature. > because the FFmpeg process of the ps -ef command is viewed as argv, That is nonsense. > Therefore, you need to copy argv to argv_copy immediately, mask the URL in the argv, > and then argv_copy is a command without mask_url for the following programs. The few milliseconds of the normal options parsing will not make a difference. > I don't understand the difference between the normal option and the mask_url in use? Your version is a special case that needs to be maintained. That is only acceptable with a very good reason, and you have not provided one. > Looking forward to your reply, thank you. Let me be clear: this here is a review of your code, not a tutorial on becoming a better developer, and even less "what an excellent idea, let us do the work for you".
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 0367930..1f6cb33 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -50,6 +50,15 @@ 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 -mask_url -i @var{url} (@emph{output}) +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. +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. +@example +ffmpeg -mask_url -i rtsp://username:password-ip:port/stream/test +@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..b40fe75 100644 --- a/doc/ffplay.texi +++ b/doc/ffplay.texi @@ -120,8 +120,16 @@ sources and sinks). Read @var{input_url}. @end table +@item -mask_url -i @var{url} (@emph{output}) +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. +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. +@end table + @section Advanced options @table @option + @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..33c0e7d 100644 --- a/doc/ffprobe.texi +++ b/doc/ffprobe.texi @@ -89,6 +89,15 @@ 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 -mask_url -i @var{url} (@emph{output}) +If the protocol address contains the user name and password, The ps -ef command exposes plaintext. +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. +@example +ffprobe -mask_url -i rtsp://username:password-ip:port/stream/test +@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..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) { + 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], "-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; 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++; + } #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); /* 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++)
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. 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. Signed-off-by: wujian_nanjing <wujian2@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(-)