* [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
@ 2022-12-21 10:10 Wujian(Chin)
2022-12-22 19:28 ` Nicolas George
0 siblings, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-21 10:10 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: wangqinghua (I)
I have modified the issues again. 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/fftools-common-opts.texi | 11 +++++++
fftools/cmdutils.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
fftools/cmdutils.h | 25 +++++++++++++++
fftools/ffmpeg.c | 10 +++---
fftools/ffplay.c | 9 ++++--
fftools/ffprobe.c | 10 +++---
6 files changed, 126 insertions(+), 14 deletions(-)
diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index d914570..77b4e4a 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -363,6 +363,17 @@ for testing. Do not use it unless you know what you're doing.
ffmpeg -cpucount 2
@end example
+@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 -max_alloc @var{bytes}
Set the maximum size limit for allocating a block on the heap by ffmpeg's
family of malloc functions. Exercise @strong{extreme caution} when using
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index a1de621..08b6c28 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -61,6 +61,69 @@ AVDictionary *format_opts, *codec_opts;
int hide_banner = 0;
+void mask_param(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 **argv_copy;
+ argv_copy = av_mallocz(argc * sizeof(char *));
+ if (!argv_copy) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy malloc failed\n");
+ exit_program(1);
+ }
+
+ for (int i = 0; i < argc; i++) {
+ int length = strlen(argv[i]) + 1;
+ argv_copy[i] = av_mallocz(length * sizeof(char *));
+ if (!argv_copy[i]) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy[%d] malloc failed\n", i);
+ exit_program(1);
+ }
+ memcpy(argv_copy[i], argv[i], length);
+ }
+ return argv_copy;
+}
+
+char **handle_arg_param(int argc, int mask_flag, char **argv)
+{
+ char **argv_copy;
+ argv_copy = copy_argv(argc, argv);
+ if (mask_flag)
+ mask_param(argc, argv);
+ return argv_copy;
+}
+
+int get_mask_flag(int *argc, char ***argv)
+{
+ if (*argc > 1 && !strcmp((*argv)[1], "-mask_url")) {
+ (*argv)[1] = (*argv)[0];
+ (*argc)--;
+ (*argv)++;
+ return 1;
+ }
+
+ return 0;
+}
+
+void free_argv_copy(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 +278,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 +295,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++) {
@@ -246,6 +309,12 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
*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..08c4da7 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -50,6 +50,31 @@ extern AVDictionary *format_opts, *codec_opts;
extern int hide_banner;
/**
+ * Using to mask sensitive info.
+ */
+void mask_param(int argc, char **argv);
+
+/**
+ * Using to copy ori argv.
+ */
+char **copy_argv(int argc, char **argv);
+
+/**
+ * Handle argv and argv_copy.
+ */
+char **handle_arg_param(int argc, int mask_flag, char **argv);
+
+/**
+ * Get mask flag.
+ */
+int get_mask_flag(int *argc, char ***argv);
+
+/**
+ * Free argv.
+ */
+void free_argv_copy(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..d16eb36 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, mask_flag;
BenchmarkTimeStamps ti;
-
+ char **argv_copy;
init_dynload();
register_exit(ffmpeg_cleanup);
@@ -3877,15 +3877,16 @@ int main(int argc, char **argv)
av_log_set_flags(AV_LOG_SKIP_REPEATED);
parse_loglevel(argc, argv, options);
+ mask_flag = get_mask_flag(&argc, &argv);
#if CONFIG_AVDEVICE
avdevice_register_all();
#endif
avformat_network_init();
show_banner(argc, argv, options);
-
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
/* parse options and open all input/output files */
- ret = ffmpeg_parse_options(argc, argv);
+ ret = ffmpeg_parse_options(argc, argv_copy);
if (ret < 0)
exit_program(1);
@@ -3920,5 +3921,6 @@ int main(int argc, char **argv)
exit_program(69);
exit_program(received_nb_signals ? 255 : main_return_code);
+ free_argv_copy(argc, argv_copy);
return main_return_code;
}
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index fc7e1c2..559e417 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3663,10 +3663,12 @@ void show_help_default(const char *opt, const char *arg)
/* Called from the main */
int main(int argc, char **argv)
{
- int flags;
+ int flags, mask_flag;
+ char **argv_copy;
VideoState *is;
init_dynload();
+ mask_flag = get_mask_flag(&argc, &argv);
av_log_set_flags(AV_LOG_SKIP_REPEATED);
parse_loglevel(argc, argv, options);
@@ -3682,7 +3684,8 @@ int main(int argc, char **argv)
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
if (!input_filename) {
show_usage();
@@ -3759,6 +3762,6 @@ int main(int argc, char **argv)
event_loop(is);
/* never returns */
-
+ free_argv_copy(argc, argv_copy);
return 0;
}
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index d2f126d..49375bd 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -4035,9 +4035,10 @@ 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, mask_flag;
+ char **argv_copy;
init_dynload();
+ mask_flag = get_mask_flag(&argc, &argv);
#if HAVE_THREADS
ret = pthread_mutex_init(&log_mutex, NULL);
@@ -4056,8 +4057,8 @@ int main(int argc, char **argv)
#endif
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
-
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
if (do_show_log)
av_log_set_callback(log_callback);
@@ -4173,6 +4174,7 @@ end:
av_freep(&print_format);
av_freep(&read_intervals);
av_hash_freep(&hash);
+ free_argv_copy(argc, argv_copy);
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-21 10:10 [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*) Wujian(Chin)
@ 2022-12-22 19:28 ` Nicolas George
2022-12-23 7:14 ` [FFmpeg-devel] 答复: " Wujian(Chin)
0 siblings, 1 reply; 28+ messages in thread
From: Nicolas George @ 2022-12-22 19:28 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 1042 bytes --]
Wujian(Chin) (12022-12-21):
> I have modified the issues again. 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/fftools-common-opts.texi | 11 +++++++
> fftools/cmdutils.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
> fftools/cmdutils.h | 25 +++++++++++++++
> fftools/ffmpeg.c | 10 +++---
> fftools/ffplay.c | 9 ++++--
> fftools/ffprobe.c | 10 +++---
> 6 files changed, 126 insertions(+), 14 deletions(-)
This new patch still has issues not addressed since the first review.
Regards,
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-22 19:28 ` Nicolas George
@ 2022-12-23 7:14 ` Wujian(Chin)
2022-12-23 9:13 ` Nicolas George
0 siblings, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-23 7:14 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
>Wujian(Chin) (12022-12-21):
>> I have modified the issues again. 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/fftools-common-opts.texi | 11 +++++++
>> fftools/cmdutils.c | 75 ++++++++++++++++++++++++++++++++++++++++++--
>> fftools/cmdutils.h | 25 +++++++++++++++
>> fftools/ffmpeg.c | 10 +++---
>> fftools/ffplay.c | 9 ++++--
>> fftools/ffprobe.c | 10 +++---
>> 6 files changed, 126 insertions(+), 14 deletions(-)
>This new patch still has issues not addressed since the first review.
>Regards,
--
> Nicolas George
I've modified most of the issues, and I've explained some of the issues that don't.
If you don't accept my explanation, do you have any other better suggestions and methods?
Thank you.
_______________________________________________
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-23 7:14 ` [FFmpeg-devel] 答复: " Wujian(Chin)
@ 2022-12-23 9:13 ` Nicolas George
2022-12-23 11:04 ` [FFmpeg-devel] 答复: " Wujian(Chin)
0 siblings, 1 reply; 28+ messages in thread
From: Nicolas George @ 2022-12-23 9:13 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 302 bytes --]
Wujian(Chin) (12022-12-23):
> I've modified most of the issues, and I've explained some of the issues that don't.
> If you don't accept my explanation, do you have any other better suggestions and methods?
I have already made a more detailed comment in the first thread.
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-23 9:13 ` Nicolas George
@ 2022-12-23 11:04 ` Wujian(Chin)
2022-12-23 11:06 ` Nicolas George
0 siblings, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-23 11:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
Other issues have been modified. The following three issues have not been modified. I have explained them:
>> @@ -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.
If you think my idea is wrong, please give your specific advice,
thank you, Nicolas George.
-----邮件原件-----
发件人: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] 代表 Nicolas George
发送时间: 2022年12月23日 17:13
收件人: 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-23):
> I've modified most of the issues, and I've explained some of the issues that don't.
> If you don't accept my explanation, do you have any other better suggestions and methods?
I have already made a more detailed comment in the first thread.
--
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] 答复: 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-23 11:04 ` [FFmpeg-devel] 答复: " Wujian(Chin)
@ 2022-12-23 11:06 ` Nicolas George
0 siblings, 0 replies; 28+ messages in thread
From: Nicolas George @ 2022-12-23 11:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 142 bytes --]
Wujian(Chin) (12022-12-23):
> If you think my idea is wrong, please give your specific advice,
I already have.
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2023-01-03 11:05 [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add " Wujian(Chin)
@ 2023-01-03 12:31 ` Nicolas George
0 siblings, 0 replies; 28+ messages in thread
From: Nicolas George @ 2023-01-03 12:31 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 226 bytes --]
Wujian(Chin) (12023-01-03):
> Please review it again, thanks!!
You still treat the option differently without a good reason.
You still do not protect credentials in options.
Not acceptable.
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
@ 2023-01-03 11:05 Wujian(Chin)
2023-01-03 12:31 ` Nicolas George
0 siblings, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2023-01-03 11:05 UTC (permalink / raw)
To: ffmpeg-devel
Please review it again, thanks!!
Signed-off-by: wujian_nanjing <wujian2@huawei.com>
---
doc/fftools-common-opts.texi | 11 +++++++++
fftools/cmdutils.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
fftools/cmdutils.h | 21 ++++++++++++++++
fftools/ffmpeg.c | 7 +++---
fftools/ffplay.c | 6 +++--
fftools/ffprobe.c | 7 +++---
fftools/opt_common.h | 1 +
7 files changed, 102 insertions(+), 8 deletions(-)
diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index d914570..724c028 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -363,6 +363,17 @@ for testing. Do not use it unless you know what you're doing.
ffmpeg -cpucount 2
@end example
+@item -mask_url -i @var{url} (@emph{output})
+If the protocol address contains the user name and password, the ps -ef
+command exposes plaintext. You can add 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 -max_alloc @var{bytes}
Set the maximum size limit for allocating a block on the heap by ffmpeg's
family of malloc functions. Exercise @strong{extreme caution} when using
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index a1de621..7946303 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -60,6 +60,59 @@ AVDictionary *swr_opts;
AVDictionary *format_opts, *codec_opts;
int hide_banner = 0;
+int mask_url = 0;
+
+void mask_param(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 **argv_copy;
+ argv_copy = av_mallocz((argc + 1) * sizeof(char *));
+ if (!argv_copy) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy malloc failed\n");
+ exit_program(1);
+ }
+
+ for (int i = 0; i < argc; i++) {
+ int length = strlen(argv[i]) + 1;
+ argv_copy[i] = av_mallocz(length * sizeof(*argv_copy));
+ if (!argv_copy[i]) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy[%d] malloc failed\n", i);
+ exit_program(1);
+ }
+ memcpy(argv_copy[i], argv[i], length);
+ }
+ argv_copy[argc] = NULL;
+ return argv_copy;
+}
+
+char **handle_arg_param(int argc, char **argv)
+{
+ char **argv_copy;
+ argv_copy = copy_argv(argc, argv);
+ if (mask_url)
+ mask_param(argc, argv);
+ return argv_copy;
+}
+
+void free_argv_copy(int argc, char **argv)
+{
+ for (int i = 0; i < argc; i++)
+ av_free(argv[i]);
+ av_free(argv);
+}
void uninit_opts(void)
{
@@ -501,6 +554,10 @@ void parse_loglevel(int argc, char **argv, const OptionDef *options)
idx = locate_option(argc, argv, options, "hide_banner");
if (idx)
hide_banner = 1;
+
+ idx = locate_option(argc, argv, options, "mask_url");
+ if (idx)
+ mask_url = 1;
}
static const AVOption *opt_find(void *obj, const char *name, const char *unit,
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 4496221..66babbd 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -48,6 +48,27 @@ extern AVDictionary *sws_dict;
extern AVDictionary *swr_opts;
extern AVDictionary *format_opts, *codec_opts;
extern int hide_banner;
+extern int mask_url;
+
+/**
+ * Using to mask sensitive info.
+ */
+void mask_param(int argc, char **argv);
+
+/**
+ * Using to copy ori argv.
+ */
+char **copy_argv(int argc, char **argv);
+
+/**
+ * Handle argv and argv_copy.
+ */
+char **handle_arg_param(int argc, char **argv);
+
+/**
+ * Free argv.
+ */
+void free_argv_copy(int argc, char **argv);
/**
* Register a program-specific cleanup routine.
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 881d6f0..9f3b261 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3867,7 +3867,7 @@ int main(int argc, char **argv)
{
int ret;
BenchmarkTimeStamps ti;
-
+ char **argv_copy;
init_dynload();
register_exit(ffmpeg_cleanup);
@@ -3883,9 +3883,10 @@ int main(int argc, char **argv)
avformat_network_init();
show_banner(argc, argv, options);
-
+ argv_copy = handle_arg_param(argc, argv);
/* parse options and open all input/output files */
- ret = ffmpeg_parse_options(argc, argv);
+ ret = ffmpeg_parse_options(argc, argv_copy);
+ free_argv_copy(argc, argv_copy);
if (ret < 0)
exit_program(1);
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index fc7e1c2..203db5e 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3664,6 +3664,7 @@ void show_help_default(const char *opt, const char *arg)
int main(int argc, char **argv)
{
int flags;
+ char **argv_copy;
VideoState *is;
init_dynload();
@@ -3682,8 +3683,9 @@ int main(int argc, char **argv)
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
-
+ argv_copy = handle_arg_param(argc, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
+ free_argv_copy(argc, argv_copy);
if (!input_filename) {
show_usage();
av_log(NULL, AV_LOG_FATAL, "An input file must be specified\n");
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index d2f126d..17e9759 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -4036,7 +4036,7 @@ int main(int argc, char **argv)
char *buf;
char *w_name = NULL, *w_args = NULL;
int ret, input_ret, i;
-
+ char **argv_copy;
init_dynload();
#if HAVE_THREADS
@@ -4056,8 +4056,8 @@ int main(int argc, char **argv)
#endif
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
-
+ argv_copy = handle_arg_param(argc, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
if (do_show_log)
av_log_set_callback(log_callback);
@@ -4173,6 +4173,7 @@ end:
av_freep(&print_format);
av_freep(&read_intervals);
av_hash_freep(&hash);
+ free_argv_copy(argc, argv_copy);
uninit_opts();
for (i = 0; i < FF_ARRAY_ELEMS(sections); i++)
diff --git a/fftools/opt_common.h b/fftools/opt_common.h
index ea1d16e..5185cf3 100644
--- a/fftools/opt_common.h
+++ b/fftools/opt_common.h
@@ -226,6 +226,7 @@ int opt_cpucount(void *optctx, const char *opt, const char *arg);
{ "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \
{ "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \
{ "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \
+ { "mask_url", OPT_BOOL, {&mask_url}, "mask the url", "flags" }, \
CMDUTILS_COMMON_OPTIONS_AVDEVICE \
#endif /* FFTOOLS_OPT_COMMON_H */
--
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-27 19:49 ` Michael Niedermayer
2022-12-28 3:20 ` [FFmpeg-devel] 答复: " Wujian(Chin)
@ 2022-12-28 8:04 ` Wujian(Chin)
1 sibling, 0 replies; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-28 8:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches
>On Mon, Dec 26, 2022 at 01:07:51PM +0000, Wujian(Chin) wrote:
>> The issue has been modified. Please review again, thank you!
>>
>> Signed-off-by: wujian_nanjing <wujian2@huawei.com>
>> ---
>> doc/fftools-common-opts.texi | 11 +++++++
>> fftools/cmdutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
>> fftools/cmdutils.h | 25 ++++++++++++++
>> fftools/ffmpeg.c | 10 +++---
>> fftools/ffplay.c | 9 ++++--
>> fftools/ffprobe.c | 10 +++---
>> 6 files changed, 128 insertions(+), 14 deletions(-)
>ffmpeg -h
>segfaults with this patch
>==32366== Invalid read of size 8
I have reproduced the problem.
The reason is that the last element of argv_copy does not end with null.
Otherwise, this problem will occur.
Thanks.
>==32366== at 0x30836B: split_commandline (in ffmpeg/ffmpeg_g)
>==32366== by 0x3039CD: ffmpeg_parse_options (in ffmpeg/ffmpeg_g)
>==32366== by 0x2ED201: main (in ffmpeg/ffmpeg_g)
>==32366== Address 0x2ced5290 is 0 bytes after a block of size 16 alloc'd
>==32366== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>==32366== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>==32366== by 0x116B322: av_malloc (in ffmpeg/ffmpeg_g)
>==32366== by 0x116B4D8: av_mallocz (in ffmpeg/ffmpeg_g)
>==32366== by 0x306469: copy_argv (in ffmpeg/ffmpeg_g)
>==32366== by 0x306537: handle_arg_param (in ffmpeg/ffmpeg_g)
>==32366== by 0x2ED1F5: main (in ffmpeg/ffmpeg_g)
>==32366==
> [...]
>--
>Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>Opposition brings concord. Out of discord comes the fairest harmony.
>-- Heraclitus
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-27 19:49 ` Michael Niedermayer
@ 2022-12-28 3:20 ` Wujian(Chin)
2022-12-28 8:04 ` Wujian(Chin)
1 sibling, 0 replies; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-28 3:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches
>ffmpeg -h
>segfaults with this patch
My environment test is OK. Details are as follows:
SZV1000266228:/usr1/wujian/build # ffmpeg -h
ffmpeg version N-109445-gcc46f5b Copyright (c) 2000-2022 the FFmpeg developers
built with gcc 7 (GCC)
configuration: --samples=/usr1/wujian/ffmpeg_master/fate-suite
libavutil 57. 43.100 / 57. 43.100
libavcodec 59. 55.103 / 59. 55.103
libavformat 59. 34.102 / 59. 34.102
libavdevice 59. 8.101 / 59. 8.101
libavfilter 8. 53.100 / 8. 53.100
libswscale 6. 8.112 / 6. 8.112
libswresample 4. 9.100 / 4. 9.100
Hyper fast Audio and Video encoder
usage: ffmpeg [options] [[infile options] -i infile]... {[outfile options] outfile}...
Getting help:
-h -- print basic options
-h long -- print more options
-h full -- print all options (including all format and codec specific options, very long)
-h type=name -- print all options for the named decoder/encoder/demuxer/muxer/filter/bsf/protocol
See man ffmpeg for detailed description of the options.
Print help / information / capabilities:
-L show license
-h topic show help
-? topic show help
-help topic show help
--help topic show help
-version show version
-buildconf show build configuration
-formats show available formats
-muxers show available muxers
-demuxers show available demuxers
-devices show available devices
-codecs show available codecs
-decoders show available decoders
-encoders show available encoders
-bsfs show available bit stream filters
-protocols show available protocols
-filters show available filters
-pix_fmts show available pixel formats
-layouts show standard channel layouts
-sample_fmts show available audio sample formats
-dispositions show available stream dispositions
-colors show available color names
-sources device list sources of the input device
-sinks device list sinks of the output device
-hwaccels show available HW acceleration methods
Global options (affect whole program instead of just one file):
-loglevel loglevel set logging level
-v loglevel set logging level
-report generate a report
-max_alloc bytes set maximum size of a single allocated block
-y overwrite output files
-n never overwrite output files
-ignore_unknown Ignore unknown stream types
-filter_threads number of non-complex filter threads
-filter_complex_threads number of threads for -filter_complex
-stats print progress report during encoding
-max_error_rate maximum error rate ratio of decoding errors (0.0: no errors, 1.0: 100% errors) above which ffmpeg returns an error instead of success.
Per-file main options:
-f fmt force format
-c codec codec name
-codec codec codec name
-pre preset preset name
-map_metadata outfile[,metadata]:infile[,metadata] set metadata information of outfile from infile
-t duration record or transcode "duration" seconds of audio/video
-to time_stop record or transcode stop time
-fs limit_size set the limit file size in bytes
-ss time_off set the start time offset
-sseof time_off set the start time offset relative to EOF
-seek_timestamp enable/disable seeking by timestamp with -ss
-timestamp time set the recording timestamp ('now' to set the current time)
-metadata string=string add metadata
-program title=string:st=number... add program with specified streams
-target type specify target file type ("vcd", "svcd", "dvd", "dv" or "dv50" with optional prefixes "pal-", "ntsc-" or "film-")
-apad audio pad
-frames number set the number of frames to output
-filter filter_graph set stream filtergraph
-filter_script filename read stream filtergraph description from a file
-reinit_filter reinit filtergraph on input parameter changes
-discard discard
-disposition disposition
Video options:
-vframes number set the number of video frames to output
-r rate set frame rate (Hz value, fraction or abbreviation)
-fpsmax rate set max frame rate (Hz value, fraction or abbreviation)
-s size set frame size (WxH or abbreviation)
-aspect aspect set aspect ratio (4:3, 16:9 or 1.3333, 1.7777)
-display_rotation angle set pure counter-clockwise rotation in degrees for stream(s)
-display_hflip set display horizontal flip for stream(s) (overrides any display rotation if it is not set)
-display_vflip set display vertical flip for stream(s) (overrides any display rotation if it is not set)
-vn disable video
-vcodec codec force video codec ('copy' to copy stream)
-timecode hh:mm:ss[:;.]ff set initial TimeCode value.
-pass n select the pass number (1 to 3)
-vf filter_graph set video filters
-ab bitrate audio bitrate (please use -b:a)
-b bitrate video bitrate (please use -b:v)
-dn disable data
Audio options:
-aframes number set the number of audio frames to output
-aq quality set audio quality (codec-specific)
-ar rate set audio sampling rate (in Hz)
-ac channels set number of audio channels
-an disable audio
-acodec codec force audio codec ('copy' to copy stream)
-af filter_graph set audio filters
Subtitle options:
-s size set frame size (WxH or abbreviation)
-sn disable subtitle
-scodec codec force subtitle codec ('copy' to copy stream)
-stag fourcc/tag force subtitle tag/fourcc
-fix_sub_duration fix subtitles duration
-canvas_size size set canvas size (WxH or abbreviation)
-spre preset set the subtitle options to the indicated preset
SZV1000266228:/usr1/wujian/build # uname -a
Linux SZV1000266228 3.0.76-0.11-default #1 SMP Fri Jun 14 08:21:43 UTC 2013 (ccab990) x86_64 x86_64 x86_64 GNU/Linux
SZV1000266228:/usr1/wujian/build # cat /etc/SuSE-release
SUSE Linux Enterprise Server 11 (x86_64)
VERSION = 11
PATCHLEVEL = 3
SZV1000266228:/usr1/wujian/build # ffmpeg -i rtsp://wwww.com -mask_url
ffmpeg version N-109445-gcc46f5b Copyright (c) 2000-2022 the FFmpeg developers
built with gcc 7 (GCC)
configuration: --samples=/usr1/wujian/ffmpeg_master/fate-suite
libavutil 57. 43.100 / 57. 43.100
libavcodec 59. 55.103 / 59. 55.103
libavformat 59. 34.102 / 59. 34.102
libavdevice 59. 8.101 / 59. 8.101
libavfilter 8. 53.100 / 8. 53.100
libswscale 6. 8.112 / 6. 8.112
libswresample 4. 9.100 / 4. 9.100
rtsp://wwww.com: Immediate exit requested
Exiting normally, received signal 2.
SZV1000266228:/usr1/wujian/build #
-----邮件原件-----
发件人: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] 代表 Michael Niedermayer
发送时间: 2022年12月28日 3:49
收件人: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
主题: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
On Mon, Dec 26, 2022 at 01:07:51PM +0000, Wujian(Chin) wrote:
> The issue has been modified. Please review again, thank you!
>
> Signed-off-by: wujian_nanjing <wujian2@huawei.com>
> ---
> doc/fftools-common-opts.texi | 11 +++++++
> fftools/cmdutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
> fftools/cmdutils.h | 25 ++++++++++++++
> fftools/ffmpeg.c | 10 +++---
> fftools/ffplay.c | 9 ++++--
> fftools/ffprobe.c | 10 +++---
> 6 files changed, 128 insertions(+), 14 deletions(-)
ffmpeg -h
segfaults with this patch
==32366== Invalid read of size 8
==32366== at 0x30836B: split_commandline (in ffmpeg/ffmpeg_g)
==32366== by 0x3039CD: ffmpeg_parse_options (in ffmpeg/ffmpeg_g)
==32366== by 0x2ED201: main (in ffmpeg/ffmpeg_g)
==32366== Address 0x2ced5290 is 0 bytes after a block of size 16 alloc'd
==32366== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32366== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32366== by 0x116B322: av_malloc (in ffmpeg/ffmpeg_g)
==32366== by 0x116B4D8: av_mallocz (in ffmpeg/ffmpeg_g)
==32366== by 0x306469: copy_argv (in ffmpeg/ffmpeg_g)
==32366== by 0x306537: handle_arg_param (in ffmpeg/ffmpeg_g)
==32366== by 0x2ED1F5: main (in ffmpeg/ffmpeg_g)
==32366==
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
[>]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-26 13:07 Wujian(Chin)
2022-12-26 13:21 ` Nicolas George
@ 2022-12-27 19:49 ` Michael Niedermayer
2022-12-28 3:20 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-28 8:04 ` Wujian(Chin)
1 sibling, 2 replies; 28+ messages in thread
From: Michael Niedermayer @ 2022-12-27 19:49 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1579 bytes --]
On Mon, Dec 26, 2022 at 01:07:51PM +0000, Wujian(Chin) wrote:
> The issue has been modified. Please review again, thank you!
>
> Signed-off-by: wujian_nanjing <wujian2@huawei.com>
> ---
> doc/fftools-common-opts.texi | 11 +++++++
> fftools/cmdutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
> fftools/cmdutils.h | 25 ++++++++++++++
> fftools/ffmpeg.c | 10 +++---
> fftools/ffplay.c | 9 ++++--
> fftools/ffprobe.c | 10 +++---
> 6 files changed, 128 insertions(+), 14 deletions(-)
ffmpeg -h
segfaults with this patch
==32366== Invalid read of size 8
==32366== at 0x30836B: split_commandline (in ffmpeg/ffmpeg_g)
==32366== by 0x3039CD: ffmpeg_parse_options (in ffmpeg/ffmpeg_g)
==32366== by 0x2ED201: main (in ffmpeg/ffmpeg_g)
==32366== Address 0x2ced5290 is 0 bytes after a block of size 16 alloc'd
==32366== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32366== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32366== by 0x116B322: av_malloc (in ffmpeg/ffmpeg_g)
==32366== by 0x116B4D8: av_mallocz (in ffmpeg/ffmpeg_g)
==32366== by 0x306469: copy_argv (in ffmpeg/ffmpeg_g)
==32366== by 0x306537: handle_arg_param (in ffmpeg/ffmpeg_g)
==32366== by 0x2ED1F5: main (in ffmpeg/ffmpeg_g)
==32366==
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-26 13:07 Wujian(Chin)
@ 2022-12-26 13:21 ` Nicolas George
2022-12-27 19:49 ` Michael Niedermayer
1 sibling, 0 replies; 28+ messages in thread
From: Nicolas George @ 2022-12-26 13:21 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 10034 bytes --]
Wujian(Chin) (12022-12-26):
> The issue has been modified. Please review again, thank you!
>
> Signed-off-by: wujian_nanjing <wujian2@huawei.com>
> ---
> doc/fftools-common-opts.texi | 11 +++++++
> fftools/cmdutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
> fftools/cmdutils.h | 25 ++++++++++++++
> fftools/ffmpeg.c | 10 +++---
> fftools/ffplay.c | 9 ++++--
> fftools/ffprobe.c | 10 +++---
> 6 files changed, 128 insertions(+), 14 deletions(-)
>
> diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
> index d914570..724c028 100644
> --- a/doc/fftools-common-opts.texi
> +++ b/doc/fftools-common-opts.texi
> @@ -363,6 +363,17 @@ for testing. Do not use it unless you know what you're doing.
> ffmpeg -cpucount 2
> @end example
>
> +@item -mask_url -i @var{url} (@emph{output})
> +If the protocol address contains the user name and password, the ps -ef
Start with what the option does.
> +command exposes plaintext. You can add 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 -max_alloc @var{bytes}
> Set the maximum size limit for allocating a block on the heap by ffmpeg's
> family of malloc functions. Exercise @strong{extreme caution} when using
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index a1de621..0f80910 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -61,6 +61,74 @@ AVDictionary *format_opts, *codec_opts;
>
> int hide_banner = 0;
>
> +void mask_param(int argc, char **argv)
> +{
> + int i, j;
> + for (i = 1; i < argc; i++) {
> + char *match = strstr(argv[i], "://");
Still leaving credentials in options visible.
> + if (match) {
> + int total = strlen(argv[i]);
> + for (j = 0; j < total; j++) {
> + argv[i][j] = '*';
> + }
> + }
> + }
> +}
> +
> +char **copy_argv(int argc, char **argv)
> +{
> + char **argv_copy;
> + argv_copy = av_mallocz(argc * sizeof(char *));
> + if (!argv_copy) {
> + av_log(NULL, AV_LOG_FATAL, "argv_copy malloc failed\n");
> + exit_program(1);
> + }
> +
> + for (int i = 0; i < argc; i++) {
> + int length = strlen(argv[i]) + 1;
> + argv_copy[i] = av_mallocz(length * sizeof(*argv_copy));
> + if (!argv_copy[i]) {
> + av_log(NULL, AV_LOG_FATAL, "argv_copy[%d] malloc failed\n", i);
> + exit_program(1);
> + }
> + memcpy(argv_copy[i], argv[i], length);
> + }
> + return argv_copy;
> +}
> +
> +char **handle_arg_param(int argc, int mask_flag, char **argv)
> +{
> + char **argv_copy;
> + argv_copy = copy_argv(argc, argv);
> + if (mask_flag)
> + mask_param(argc, argv);
> + return argv_copy;
> +}
> +
> +int get_mask_flag(int *argc, char ***argv)
> +{
> + for (int i = 1; i < *argc; i++) {
> + if (strcmp((*argv)[i], "-mask_url")) {
> + continue;
> + }
> +
> + for (int j = i + 1; j < *argc; j++) {
> + (*argv)[j - 1] = (*argv)[j];
> + }
> + (*argc)--;
> + return 1;
> + }
> +
> + return 0;
> +}
Still unacceptable.
> +
> +void free_argv_copy(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 +283,16 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
> if (win32_argv_utf8) {
> *argc_ptr = win32_argc;
> *argv_ptr = win32_argv_utf8;
> + get_mask_flag(argc_ptr, argv_ptr);
> return;
> }
>
> win32_argc = 0;
> argv_w = CommandLineToArgvW(GetCommandLineW(), &win32_argc);
> - if (win32_argc <= 0 || !argv_w)
> + if (win32_argc <= 0 || !argv_w) {
> + get_mask_flag(argc_ptr, argv_ptr);
> return;
> + }
>
> /* determine the UTF-8 buffer size (including NULL-termination symbols) */
> for (i = 0; i < win32_argc; i++)
> @@ -232,6 +303,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);
> + get_mask_flag(argc_ptr, argv_ptr);
> return;
> }
>
> @@ -246,6 +318,7 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
>
> *argc_ptr = win32_argc;
> *argv_ptr = win32_argv_utf8;
> + get_mask_flag(argc_ptr, argv_ptr);
> }
> #else
> static inline void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
> @@ -696,10 +769,8 @@ int split_commandline(OptionParseContext *octx, int argc, char *argv[],
> {
> int optindex = 1;
> int dashdash = -2;
> -
> /* perform system-dependent conversions for arguments list */
> prepare_app_arguments(&argc, &argv);
> -
> init_parse_context(octx, groups, nb_groups);
> av_log(NULL, AV_LOG_DEBUG, "Splitting the commandline.\n");
>
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 4496221..08c4da7 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -50,6 +50,31 @@ extern AVDictionary *format_opts, *codec_opts;
> extern int hide_banner;
>
> /**
> + * Using to mask sensitive info.
> + */
> +void mask_param(int argc, char **argv);
> +
> +/**
> + * Using to copy ori argv.
> + */
> +char **copy_argv(int argc, char **argv);
> +
> +/**
> + * Handle argv and argv_copy.
> + */
> +char **handle_arg_param(int argc, int mask_flag, char **argv);
> +
> +/**
> + * Get mask flag.
> + */
> +int get_mask_flag(int *argc, char ***argv);
> +
> +/**
> + * Free argv.
> + */
> +void free_argv_copy(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..d16eb36 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, mask_flag;
> BenchmarkTimeStamps ti;
> -
> + char **argv_copy;
> init_dynload();
>
> register_exit(ffmpeg_cleanup);
> @@ -3877,15 +3877,16 @@ int main(int argc, char **argv)
> av_log_set_flags(AV_LOG_SKIP_REPEATED);
> parse_loglevel(argc, argv, options);
>
> + mask_flag = get_mask_flag(&argc, &argv);
> #if CONFIG_AVDEVICE
> avdevice_register_all();
> #endif
> avformat_network_init();
>
> show_banner(argc, argv, options);
> -
> + argv_copy = handle_arg_param(argc, mask_flag, argv);
> /* parse options and open all input/output files */
> - ret = ffmpeg_parse_options(argc, argv);
> + ret = ffmpeg_parse_options(argc, argv_copy);
> if (ret < 0)
> exit_program(1);
>
> @@ -3920,5 +3921,6 @@ int main(int argc, char **argv)
> exit_program(69);
>
> exit_program(received_nb_signals ? 255 : main_return_code);
> + free_argv_copy(argc, argv_copy);
> return main_return_code;
> }
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index fc7e1c2..559e417 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -3663,10 +3663,12 @@ void show_help_default(const char *opt, const char *arg)
> /* Called from the main */
> int main(int argc, char **argv)
> {
> - int flags;
> + int flags, mask_flag;
> + char **argv_copy;
> VideoState *is;
>
> init_dynload();
> + mask_flag = get_mask_flag(&argc, &argv);
>
> av_log_set_flags(AV_LOG_SKIP_REPEATED);
> parse_loglevel(argc, argv, options);
> @@ -3682,7 +3684,8 @@ int main(int argc, char **argv)
>
> show_banner(argc, argv, options);
>
> - parse_options(NULL, argc, argv, options, opt_input_file);
> + argv_copy = handle_arg_param(argc, mask_flag, argv);
> + parse_options(NULL, argc, argv_copy, options, opt_input_file);
>
> if (!input_filename) {
> show_usage();
> @@ -3759,6 +3762,6 @@ int main(int argc, char **argv)
> event_loop(is);
>
> /* never returns */
> -
> + free_argv_copy(argc, argv_copy);
> return 0;
> }
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index d2f126d..49375bd 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -4035,9 +4035,10 @@ 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, mask_flag;
> + char **argv_copy;
> init_dynload();
> + mask_flag = get_mask_flag(&argc, &argv);
>
> #if HAVE_THREADS
> ret = pthread_mutex_init(&log_mutex, NULL);
> @@ -4056,8 +4057,8 @@ int main(int argc, char **argv)
> #endif
>
> show_banner(argc, argv, options);
> - parse_options(NULL, argc, argv, options, opt_input_file);
> -
> + argv_copy = handle_arg_param(argc, mask_flag, argv);
> + parse_options(NULL, argc, argv_copy, options, opt_input_file);
> if (do_show_log)
> av_log_set_callback(log_callback);
>
> @@ -4173,6 +4174,7 @@ end:
> av_freep(&print_format);
> av_freep(&read_intervals);
> av_hash_freep(&hash);
> + free_argv_copy(argc, argv_copy);
>
> uninit_opts();
> for (i = 0; i < FF_ARRAY_ELEMS(sections); i++)
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
@ 2022-12-26 13:07 Wujian(Chin)
2022-12-26 13:21 ` Nicolas George
2022-12-27 19:49 ` Michael Niedermayer
0 siblings, 2 replies; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-26 13:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: wangqinghua (I)
The issue has been modified. Please review again, thank you!
Signed-off-by: wujian_nanjing <wujian2@huawei.com>
---
doc/fftools-common-opts.texi | 11 +++++++
fftools/cmdutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++--
fftools/cmdutils.h | 25 ++++++++++++++
fftools/ffmpeg.c | 10 +++---
fftools/ffplay.c | 9 ++++--
fftools/ffprobe.c | 10 +++---
6 files changed, 128 insertions(+), 14 deletions(-)
diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index d914570..724c028 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -363,6 +363,17 @@ for testing. Do not use it unless you know what you're doing.
ffmpeg -cpucount 2
@end example
+@item -mask_url -i @var{url} (@emph{output})
+If the protocol address contains the user name and password, the ps -ef
+command exposes plaintext. You can add 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 -max_alloc @var{bytes}
Set the maximum size limit for allocating a block on the heap by ffmpeg's
family of malloc functions. Exercise @strong{extreme caution} when using
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index a1de621..0f80910 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -61,6 +61,74 @@ AVDictionary *format_opts, *codec_opts;
int hide_banner = 0;
+void mask_param(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 **argv_copy;
+ argv_copy = av_mallocz(argc * sizeof(char *));
+ if (!argv_copy) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy malloc failed\n");
+ exit_program(1);
+ }
+
+ for (int i = 0; i < argc; i++) {
+ int length = strlen(argv[i]) + 1;
+ argv_copy[i] = av_mallocz(length * sizeof(*argv_copy));
+ if (!argv_copy[i]) {
+ av_log(NULL, AV_LOG_FATAL, "argv_copy[%d] malloc failed\n", i);
+ exit_program(1);
+ }
+ memcpy(argv_copy[i], argv[i], length);
+ }
+ return argv_copy;
+}
+
+char **handle_arg_param(int argc, int mask_flag, char **argv)
+{
+ char **argv_copy;
+ argv_copy = copy_argv(argc, argv);
+ if (mask_flag)
+ mask_param(argc, argv);
+ return argv_copy;
+}
+
+int get_mask_flag(int *argc, char ***argv)
+{
+ for (int i = 1; i < *argc; i++) {
+ if (strcmp((*argv)[i], "-mask_url")) {
+ continue;
+ }
+
+ for (int j = i + 1; j < *argc; j++) {
+ (*argv)[j - 1] = (*argv)[j];
+ }
+ (*argc)--;
+ return 1;
+ }
+
+ return 0;
+}
+
+void free_argv_copy(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 +283,16 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
if (win32_argv_utf8) {
*argc_ptr = win32_argc;
*argv_ptr = win32_argv_utf8;
+ get_mask_flag(argc_ptr, argv_ptr);
return;
}
win32_argc = 0;
argv_w = CommandLineToArgvW(GetCommandLineW(), &win32_argc);
- if (win32_argc <= 0 || !argv_w)
+ if (win32_argc <= 0 || !argv_w) {
+ get_mask_flag(argc_ptr, argv_ptr);
return;
+ }
/* determine the UTF-8 buffer size (including NULL-termination symbols) */
for (i = 0; i < win32_argc; i++)
@@ -232,6 +303,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);
+ get_mask_flag(argc_ptr, argv_ptr);
return;
}
@@ -246,6 +318,7 @@ static void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
*argc_ptr = win32_argc;
*argv_ptr = win32_argv_utf8;
+ get_mask_flag(argc_ptr, argv_ptr);
}
#else
static inline void prepare_app_arguments(int *argc_ptr, char ***argv_ptr)
@@ -696,10 +769,8 @@ int split_commandline(OptionParseContext *octx, int argc, char *argv[],
{
int optindex = 1;
int dashdash = -2;
-
/* perform system-dependent conversions for arguments list */
prepare_app_arguments(&argc, &argv);
-
init_parse_context(octx, groups, nb_groups);
av_log(NULL, AV_LOG_DEBUG, "Splitting the commandline.\n");
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 4496221..08c4da7 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -50,6 +50,31 @@ extern AVDictionary *format_opts, *codec_opts;
extern int hide_banner;
/**
+ * Using to mask sensitive info.
+ */
+void mask_param(int argc, char **argv);
+
+/**
+ * Using to copy ori argv.
+ */
+char **copy_argv(int argc, char **argv);
+
+/**
+ * Handle argv and argv_copy.
+ */
+char **handle_arg_param(int argc, int mask_flag, char **argv);
+
+/**
+ * Get mask flag.
+ */
+int get_mask_flag(int *argc, char ***argv);
+
+/**
+ * Free argv.
+ */
+void free_argv_copy(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..d16eb36 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, mask_flag;
BenchmarkTimeStamps ti;
-
+ char **argv_copy;
init_dynload();
register_exit(ffmpeg_cleanup);
@@ -3877,15 +3877,16 @@ int main(int argc, char **argv)
av_log_set_flags(AV_LOG_SKIP_REPEATED);
parse_loglevel(argc, argv, options);
+ mask_flag = get_mask_flag(&argc, &argv);
#if CONFIG_AVDEVICE
avdevice_register_all();
#endif
avformat_network_init();
show_banner(argc, argv, options);
-
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
/* parse options and open all input/output files */
- ret = ffmpeg_parse_options(argc, argv);
+ ret = ffmpeg_parse_options(argc, argv_copy);
if (ret < 0)
exit_program(1);
@@ -3920,5 +3921,6 @@ int main(int argc, char **argv)
exit_program(69);
exit_program(received_nb_signals ? 255 : main_return_code);
+ free_argv_copy(argc, argv_copy);
return main_return_code;
}
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index fc7e1c2..559e417 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3663,10 +3663,12 @@ void show_help_default(const char *opt, const char *arg)
/* Called from the main */
int main(int argc, char **argv)
{
- int flags;
+ int flags, mask_flag;
+ char **argv_copy;
VideoState *is;
init_dynload();
+ mask_flag = get_mask_flag(&argc, &argv);
av_log_set_flags(AV_LOG_SKIP_REPEATED);
parse_loglevel(argc, argv, options);
@@ -3682,7 +3684,8 @@ int main(int argc, char **argv)
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
if (!input_filename) {
show_usage();
@@ -3759,6 +3762,6 @@ int main(int argc, char **argv)
event_loop(is);
/* never returns */
-
+ free_argv_copy(argc, argv_copy);
return 0;
}
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index d2f126d..49375bd 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -4035,9 +4035,10 @@ 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, mask_flag;
+ char **argv_copy;
init_dynload();
+ mask_flag = get_mask_flag(&argc, &argv);
#if HAVE_THREADS
ret = pthread_mutex_init(&log_mutex, NULL);
@@ -4056,8 +4057,8 @@ int main(int argc, char **argv)
#endif
show_banner(argc, argv, options);
- parse_options(NULL, argc, argv, options, opt_input_file);
-
+ argv_copy = handle_arg_param(argc, mask_flag, argv);
+ parse_options(NULL, argc, argv_copy, options, opt_input_file);
if (do_show_log)
av_log_set_callback(log_callback);
@@ -4173,6 +4174,7 @@ end:
av_freep(&print_format);
av_freep(&read_intervals);
av_hash_freep(&hash);
+ free_argv_copy(argc, argv_copy);
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] 答复: 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-24 8:51 ` [FFmpeg-devel] 答复: " Wujian(Chin)
@ 2022-12-24 8:59 ` Nicolas George
0 siblings, 0 replies; 28+ messages in thread
From: Nicolas George @ 2022-12-24 8:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 1566 bytes --]
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".
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-22 19:27 ` Nicolas George
@ 2022-12-24 8:51 ` Wujian(Chin)
2022-12-24 8:59 ` Nicolas George
0 siblings, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-24 8:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 14:51 ` [FFmpeg-devel] " "zhilizhao(赵志立)"
@ 2022-12-22 23:14 ` Marton Balint
0 siblings, 0 replies; 28+ messages in thread
From: Marton Balint @ 2022-12-22 23:14 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-20 11:42 ` [FFmpeg-devel] 答复: " Wujian(Chin)
@ 2022-12-22 19:27 ` Nicolas George
2022-12-24 8:51 ` [FFmpeg-devel] 答复: " Wujian(Chin)
0 siblings, 1 reply; 28+ messages in thread
From: Nicolas George @ 2022-12-22 19:27 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
[-- Attachment #1.1: Type: text/plain, Size: 735 bytes --]
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,
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:45 ` Nicolas George
@ 2022-12-20 11:56 ` Wujian(Chin)
0 siblings, 0 replies; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-20 11:56 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
>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
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] 答复: [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:30 ` Nicolas George
2022-12-19 13:37 ` Gyan Doshi
@ 2022-12-20 11:42 ` Wujian(Chin)
2022-12-22 19:27 ` Nicolas George
1 sibling, 1 reply; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-20 11:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
>> @@ -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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:40 ` Marvin Scholz
2022-12-19 13:45 ` Nicolas George
@ 2022-12-19 14:51 ` "zhilizhao(赵志立)"
2022-12-22 23:14 ` Marton Balint
1 sibling, 1 reply; 28+ messages in thread
From: "zhilizhao(赵志立)" @ 2022-12-19 14:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:40 ` Marvin Scholz
@ 2022-12-19 13:45 ` Nicolas George
2022-12-20 11:56 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-19 14:51 ` [FFmpeg-devel] " "zhilizhao(赵志立)"
1 sibling, 1 reply; 28+ messages in thread
From: Nicolas George @ 2022-12-19 13:45 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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,
--
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:37 ` Gyan Doshi
@ 2022-12-19 13:44 ` Nicolas George
0 siblings, 0 replies; 28+ messages in thread
From: Nicolas George @ 2022-12-19 13:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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,
--
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:37 ` Nicolas George
@ 2022-12-19 13:40 ` Marvin Scholz
2022-12-19 13:45 ` Nicolas George
2022-12-19 14:51 ` [FFmpeg-devel] " "zhilizhao(赵志立)"
0 siblings, 2 replies; 28+ messages in thread
From: Marvin Scholz @ 2022-12-19 13:40 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:33 ` [FFmpeg-devel] " Marvin Scholz
@ 2022-12-19 13:37 ` Nicolas George
2022-12-19 13:40 ` Marvin Scholz
0 siblings, 1 reply; 28+ messages in thread
From: Nicolas George @ 2022-12-19 13:37 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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,
--
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:30 ` Nicolas George
@ 2022-12-19 13:37 ` Gyan Doshi
2022-12-19 13:44 ` Nicolas George
2022-12-20 11:42 ` [FFmpeg-devel] 答复: " Wujian(Chin)
1 sibling, 1 reply; 28+ messages in thread
From: Gyan Doshi @ 2022-12-19 13:37 UTC (permalink / raw)
To: ffmpeg-devel
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
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:15 [FFmpeg-devel] " Wujian(Chin)
2022-12-19 13:30 ` Nicolas George
@ 2022-12-19 13:33 ` Marvin Scholz
2022-12-19 13:37 ` Nicolas George
1 sibling, 1 reply; 28+ messages in thread
From: Marvin Scholz @ 2022-12-19 13:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
2022-12-19 13:15 [FFmpeg-devel] " Wujian(Chin)
@ 2022-12-19 13:30 ` Nicolas George
2022-12-19 13:37 ` Gyan Doshi
2022-12-20 11:42 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-19 13:33 ` [FFmpeg-devel] " Marvin Scholz
1 sibling, 2 replies; 28+ messages in thread
From: Nicolas George @ 2022-12-19 13:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: wangqinghua (I)
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)
@ 2022-12-19 13:15 Wujian(Chin)
2022-12-19 13:30 ` Nicolas George
2022-12-19 13:33 ` [FFmpeg-devel] " Marvin Scholz
0 siblings, 2 replies; 28+ messages in thread
From: Wujian(Chin) @ 2022-12-19 13:15 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: wangqinghua (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(-)
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++)
--
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".
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2023-01-03 12:31 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-21 10:10 [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils:add -mask_url to replace the protocol address in the command with the asterisk (*) Wujian(Chin)
2022-12-22 19:28 ` Nicolas George
2022-12-23 7:14 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-23 9:13 ` Nicolas George
2022-12-23 11:04 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-23 11:06 ` Nicolas George
-- strict thread matches above, loose matches on Subject: below --
2023-01-03 11:05 [FFmpeg-devel] [PATCH] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add " Wujian(Chin)
2023-01-03 12:31 ` Nicolas George
2022-12-26 13:07 Wujian(Chin)
2022-12-26 13:21 ` Nicolas George
2022-12-27 19:49 ` Michael Niedermayer
2022-12-28 3:20 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-28 8:04 ` Wujian(Chin)
2022-12-19 13:15 [FFmpeg-devel] " Wujian(Chin)
2022-12-19 13:30 ` Nicolas George
2022-12-19 13:37 ` Gyan Doshi
2022-12-19 13:44 ` Nicolas George
2022-12-20 11:42 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-22 19:27 ` Nicolas George
2022-12-24 8:51 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-24 8:59 ` Nicolas George
2022-12-19 13:33 ` [FFmpeg-devel] " Marvin Scholz
2022-12-19 13:37 ` Nicolas George
2022-12-19 13:40 ` Marvin Scholz
2022-12-19 13:45 ` Nicolas George
2022-12-20 11:56 ` [FFmpeg-devel] 答复: " Wujian(Chin)
2022-12-19 14:51 ` [FFmpeg-devel] " "zhilizhao(赵志立)"
2022-12-22 23:14 ` Marton Balint
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git