[FFmpeg-devel] [PATCH] libswcale: Fix possible string overflow in test

Adam Richter adamrichter4 at gmail.com
Mon May 13 21:32:37 EEST 2019


Hi, Michael.

Thanks for reviewing that possible string overflow found by cppcheck
and proposing to try to make a better fix.  I'll assume no further
action on my part for this fix is necessary unless anyone tells me
otherwise.

Adam


Adam

On Mon, May 13, 2019 at 4:39 AM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Sun, May 12, 2019 at 05:40:00AM -0700, Adam Richter wrote:
> > This is a possible fix for a string overflow in some sscanf calls in
> > libswcale/tests/swscale.c, in the function fileTest(), found by
> > cppcheck.  Please see the attachment for more discussion of this.
> >
> > Thanks in advance for considering this patch.
> >
> > Adam
>
> >  swscale.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 337bfa52e3917c2d896ca5c7ba1b669d5970cdab  0002-libswcale-Fix-possible-string-overflow-in-test.patch
> > From 8b5f994bcd2576588149f228695823b5cf8d3dc8 Mon Sep 17 00:00:00 2001
> > From: Adam Richter <adamrichter4 at gmail.com>
> > Date: Sun, 12 May 2019 05:03:25 -0700
> > Subject: [PATCH] libswcale: Fix possible string overflow in test.
> >
> > In libswcale/tests/swcale.c, the function fileTest() calls sscanf in
> > an argument of "%12s" on character srcStr[] and dstStr[], which are
> > only 12 bytes.  So, if the input string is 12 characters, a
> > terminating null byte can be written past the end of these arrays.
> >
> > This bug was found by cppcheck.
> >
> > I am not an ffmpeg or libswcale developer, and I believe that this is
> > the first patch I am submitting to ffmpeg, so please let me know if
> > I am doing anything wrong in the patch submission process.
> >
> > For the same reason, please examine this patch skeptically, especially
> > considering that I have not tested this patch other than to see that
> > it compiled without complaint and that "make fate" completed with a
> > zero exit code.  I do not know if this program actually
> > expects these input strings to be a maximum of 11 or 12 characters long.
> > In this patch, I assume that they could be 12 characters long, so I have
> > extended the array sizes, but perhaps a more correct fix might
> > be to change the "%12s" instances to "%11s" instead.
> >
> > Thanks in advance for considering this patch.
>
> I actually think 13 is not long enough for the longest name.
> Ill fix it, thanks for finding this
>
>
> [...]
>
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Rewriting code that is poorly written but fully understood is good.
> Rewriting code that one doesnt understand is a sign that one is less smart
> then the original author, trying to rewrite it will not make it better.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list