diff mbox series

[FFmpeg-devel] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*)

Message ID e4bdc78745ed4218a6f2505af533db4b@huawei.com
State New
Headers show
Series [FFmpeg-devel] fftools/ffmpeg_ffplay_ffprobe_cmdutils: add -mask_url to replace the protocol address in the command with the asterisk (*) | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Wujian(Chin) Dec. 26, 2022, 1:07 p.m. UTC 
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(-)

Comments

Nicolas George Dec. 26, 2022, 1:21 p.m. UTC | #1 
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++)
Michael Niedermayer Dec. 27, 2022, 7:49 p.m. UTC | #2 
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== 

[...]
Wujian(Chin) Dec. 28, 2022, 3:20 a.m. UTC | #3 
>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".
Wujian(Chin) Dec. 28, 2022, 8:04 a.m. UTC | #4 
>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
diff mbox series

Patch

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++)