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

[Aaron Boushley]

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.

-------------- next part --------------
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");
     }
 
     return 0;
@@ -1361,7 +1364,7 @@ static int draw_text(AVFilterContext *ctx, AVFrame *frame,
 
     /* load and cache glyphs */
     for (i = 0, p = text; *p; i++) {
-        GET_UTF8(code, *p++, continue;);
+        GET_UTF8(code, *p++, goto invalid_caching;);
 
         /* get glyph */
         dummy.code = code;
@@ -1377,6 +1380,10 @@ static int draw_text(AVFilterContext *ctx, AVFrame *frame,
         y_max = FFMAX(glyph->bbox.yMax, y_max);
         x_min = FFMIN(glyph->bbox.xMin, x_min);
         x_max = FFMAX(glyph->bbox.xMax, x_max);
+
+        continue;
+invalid_caching:
+        av_log(ctx, AV_LOG_DEBUG, "Invalid UTF8 character while caching glyphs\n");
     }
     s->max_glyph_h = y_max - y_min;
     s->max_glyph_w = x_max - x_min;
@@ -1384,7 +1391,7 @@ static int draw_text(AVFilterContext *ctx, AVFrame *frame,
     /* compute and save position for each glyph */
     glyph = NULL;
     for (i = 0, p = text; *p; i++) {
-        GET_UTF8(code, *p++, continue;);
+        GET_UTF8(code, *p++, goto invalid_positioning;);
 
         /* skip the \n in the sequence \r\n */
         if (prev_code == '\r' && code == '\n')
@@ -1417,6 +1424,10 @@ static int draw_text(AVFilterContext *ctx, AVFrame *frame,
         s->positions[i].y = y - glyph->bitmap_top + y_max;
         if (code == '\t') x  = (x / s->tabsize + 1)*s->tabsize;
         else              x += glyph->advance;
+
+        continue;
+invalid_positioning:
+        av_log(ctx, AV_LOG_DEBUG, "Invalid UTF8 character while positioning glyphs\n");
     }
 
     max_text_line_w = FFMAX(x, max_text_line_w);
-- 
2.20.1 (Apple Git-117)