[FFmpeg-devel] [PATCH] libavfilter/vf_drawtext: Avoid undefined behavior from GET_UTF8

[Aaron Boushley]

I'm open to suggestions of alternatives.

I want sure if I could do a goto if there was no code at that location so I
chose to add a log statement figuring that would be helpful if you had
problems.

I chose debug because previously the character was completely ignored, so
didn't want to go from no notification to an error log.

I could also just put a continue call there, or do more research to figure
out if I can go to without having any content on the line.

Suggestions for what I should do differently with that context?

Aaron

On Mon, Sep 9, 2019, 9:13 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Sat, Jul 27, 2019 at 07:58:48AM -0700, Aaron Boushley wrote:
> > The vf_drawtext filter uses the GET_UTF8 macro in multiple locations.
> > Each of these use `continue;` as the error handler. However the
> > documentation for the GET_UTF8 macro states "ERROR should not contain
> > a loop control statement which could interact with the internal while
> > loop, and should force an exit from the macro code (e.g. through a
> > goto or a return) in order to prevent undefined results."
> >
> > This patch adjusts vf_drawtext to use goto error handlers similar to
> > other locations in ffmpeg.
> >
> > Aaron
> >
> > PS Sorry for having to send again, sent from the wrong address last
> > time, so patchwork didn't pick it up.
> >
>
> >  vf_drawtext.c |   17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 04550ed40f98ec78c2719de31da889e92ff42f45
> libavfilter-drawtext-avoid-undefined-behavior.patch
> > From efdc96ace59d676e76434499a399d1d7df7fa093 Mon Sep 17 00:00:00 2001
> > From: Aaron Boushley <boushley at gmail.com>
> > Date: Fri, 26 Jul 2019 15:49:36 -0700
> > Subject: [PATCH] libavfilter/drawtext: avoid undefined behavior with
> GET_UTF8
> >
> > Currently the GET_UTF8 usage in drawtext use a continue to skip invalid
> > characters in a string. The macro definition states that using a loop
> > control statement results in undefined behavior since the macro itself
> > uses a loop.
> >
> > This switches drawtext to use a goto statement similar to other usages
> > of GET_UTF8 in other parts of ffmpeg.
> >
> > Signed-off-by: Aaron Boushley <boushley at gmail.com>
> > ---
> >  libavfilter/vf_drawtext.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> > index 8f4badbdb5..fd2ba84d12 100644
> > --- a/libavfilter/vf_drawtext.c
> > +++ b/libavfilter/vf_drawtext.c
> > @@ -1223,7 +1223,7 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> >      for (i = 0, p = text; *p; i++) {
> >          FT_Bitmap bitmap;
> >          Glyph dummy = { 0 };
> > -        GET_UTF8(code, *p++, continue;);
> > +        GET_UTF8(code, *p++, goto invalid_drawing;);
> >
> >          /* skip new line chars, just go to new line */
> >          if (code == '\n' || code == '\r' || code == '\t')
> > @@ -1248,6 +1248,9 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> >                        bitmap.width, bitmap.rows,
> >                        bitmap.pixel_mode == FT_PIXEL_MODE_MONO ? 0 : 3,
> >                        0, x1, y1);
> > +        continue;
> > +invalid_drawing:
> > +        av_log(s, AV_LOG_DEBUG, "Invalid UTF8 character while drawing
> glyphs\n");
> >      }
>
> Why does this add av_log() calls ?
> This seems unrelated to the bug fix. Also why are they debug level ?
> if a user draws a string she provided that could be argued to be an error
> the user wants to know about. But maybe iam missing something
> Also they probably should provide more information about where the
> invalid character is. It may be hard to find in a long text for some users
>
> Thanks
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The educated differ from the uneducated as much as the living from the
> dead. -- Aristotle
> _______________________________________________
> 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".