[FFmpeg-devel,V2,6/7] libavformat/smoothstreamingenc.c: fix build warning for [-Wformat-truncation=]

Part of the build message:
src/libavformat/smoothstreamingenc.c: In function ‘ism_flush’:
src/libavformat/smoothstreamingenc.c:510:49: warning: ‘/temp’ directive output may be truncated writing 5 bytes into a region of size between 1 and 1024 [-Wformat-truncation=]
         snprintf(filename, sizeof(filename), "%s/temp", os->dirname);
                                                 ^~~~~
src/libavformat/smoothstreamingenc.c:510:9: note: ‘snprintf’ output between 6 and 1029 bytes into a destination of size 1024
         snprintf(filename, sizeof(filename), "%s/temp", os->dirname);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libavformat/smoothstreamingenc.c:538:53: warning: ‘/temp’ directive output may be truncated writing 5 bytes into a region of size between 1 and 1024 [-Wformat-truncation=]
             snprintf(filename, sizeof(filename), "%s/temp", os->dirname);
                                                     ^~~~~
src/libavformat/smoothstreamingenc.c:538:13: note: ‘snprintf’ output between 6 and 1029 bytes into a destination of size 1024
             snprintf(filename, sizeof(filename), "%s/temp", os->dirname);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libavformat/smoothstreamingenc.c:545:63: warning: ‘/FragmentInfo(’ directive output may be truncated writing 14 bytes into a region of size between 1 and 1024 [-Wformat-truncation=]
         snprintf(header_filename, sizeof(header_filename), "%s/FragmentInfo(%s=%"PRIu64")", os->dirname, os->stream_type_tag, start_ts);
                                                               ^~~~~~~~~~~~~~
src/libavformat/smoothstreamingenc.c:545:60: note: using the range [0, 18446744073709551615] for directive argument
         snprintf(header_filename, sizeof(header_filename), "%s/FragmentInfo(%s=%"PRIu64")", os->dirname, os->stream_type_tag, start_ts);
                                                            ^~~~~~~~~~~~~~~~~~~~~~
src/libavformat/smoothstreamingenc.c:545:9: note: ‘snprintf’ output 18 or more bytes (assuming 1041) into a destination of size 1024
         snprintf(header_filename, sizeof(header_filename), "%s/FragmentInfo(%s=%"PRIu64")", os->dirname, os->stream_type_tag, start_ts);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
---
 libavformat/smoothstreamingenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

> On 25 Feb 2021, at 07:38, Guo, Yejun <yejun.guo@intel.com> wrote:
> --- a/libavformat/smoothstreamingenc.c
> +++ b/libavformat/smoothstreamingenc.c
> @@ -501,7 +501,7 @@ static int ism_flush(AVFormatContext *s, int final)
> 
>     for (i = 0; i < s->nb_streams; i++) {
>         OutputStream *os = &c->streams[i];
> -        char filename[1024], target_filename[1024], header_filename[1024], curr_dirname[1024];
> +        char filename[2048], target_filename[2048], header_filename[2048], curr_dirname[1024];
>         int64_t size;
>         int64_t start_ts, duration, moof_size;
>         if (!os->packets_written)

IMO some of these allocations are getting a bit too large for the stack
(multi-page stack allocations weaken security measures even if the large
arrays themselves do not overflow).
And no matter what size you put, there’s always a larger filename possible
where it breaks, so it feels like just warning polishing with marginal
real benefit.
Why not use av_asprintf, then at least the problem is actually solved for real?
I don’t see that this code is performance relevant, so the only reason to
put these onto stack is being too lazy to do the memory management, which
I think is a fairly weak argument.

Best regards,
Reimar

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Reimar
> D?ffinger
> Sent: 2021年2月25日 16:22
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V2 6/7]
> libavformat/smoothstreamingenc.c: fix build warning for
> [-Wformat-truncation=]
> 
> 
> > On 25 Feb 2021, at 07:38, Guo, Yejun <yejun.guo@intel.com> wrote:
> > --- a/libavformat/smoothstreamingenc.c
> > +++ b/libavformat/smoothstreamingenc.c
> > @@ -501,7 +501,7 @@ static int ism_flush(AVFormatContext *s, int
> > final)
> >
> >     for (i = 0; i < s->nb_streams; i++) {
> >         OutputStream *os = &c->streams[i];
> > -        char filename[1024], target_filename[1024],
> header_filename[1024], curr_dirname[1024];
> > +        char filename[2048], target_filename[2048],
> > + header_filename[2048], curr_dirname[1024];
> >         int64_t size;
> >         int64_t start_ts, duration, moof_size;
> >         if (!os->packets_written)
> 
> IMO some of these allocations are getting a bit too large for the stack
> (multi-page stack allocations weaken security measures even if the large arrays
> themselves do not overflow).
> And no matter what size you put, there’s always a larger filename possible
> where it breaks, so it feels like just warning polishing with marginal real
> benefit.
> Why not use av_asprintf, then at least the problem is actually solved for real?
> I don’t see that this code is performance relevant, so the only reason to put
> these onto stack is being too lazy to do the memory management, which I
> think is a fairly weak argument.
> 

thanks, and get the point that it is a possible security issue.

There's a max len for the filename, and so I use 'filename[1029] etc.' in V1 patch.
But I think the magic number 1029 is weird and so just changed to 2048 in V2.
There's no other reasons to choose 2048.

av_asprintf is a good choice, I'll look at it and send patches in V3.