parse: avoid signed overflow for effect_skip_timing

Discovered by OSS-Fuzz.

For sufficiently large valued or numerous karaoke tags
the addition to effect_skip_timing can overflow.
Since the operands were signed, this was undefined behaviour.

For utmost VSFilter compatibility, we'd like to ideally emulate its
effective wraparound behaviour. Although the exact order pf calculation
differs, merely ensuring our values safely wraparound too should yield
the same end result.

To that end, we first change the types of the relevant members to
int32_t guaranteeing the same size as in VSFilter and complement-of-two
representation and then also cast one operand to uint32_t.
Through integer promotion both operands will be treated as unsigned,
so any overflow will be (perfectly defined) unsigend overflow.
The final implicit cast back to int32_t is technically implementation
defined and may raise an "implementation-defined signal", but due to
the complement-of-two semantics in pratice we just get a wraparound
matching the wraparaound expected for the original signed
complement-of-two overflow, but while avoiding UB.
Compilers appear to emit the same machine code on x86 with or without
the uint32_t cast, if any optimisations are enabled and otherwise
at most minor differences.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=47993

2 files changed, 10 insertions, 10 deletions

diff --git a/libass/ass_parse.c b/libass/ass_parse.c
index 0b30b68..f43831e 100644
--- a/libass/ass_parse.c
+++ b/libass/ass_parse.c
@@ -796,7 +796,7 @@ char *parse_tags(ASS_Renderer *render_priv, char *p, char *end, double pwr,
                 val = argtod(*args);
             render_priv->state.effect_type = EF_KARAOKE_KF;
             render_priv->state.effect_skip_timing +=
-                    render_priv->state.effect_timing;
+                    (uint32_t) render_priv->state.effect_timing;
             render_priv->state.effect_timing = dtoi32(val * 10);
         } else if (tag("ko")) {
             double val = 100;
@@ -804,7 +804,7 @@ char *parse_tags(ASS_Renderer *render_priv, char *p, char *end, double pwr,
                 val = argtod(*args);
             render_priv->state.effect_type = EF_KARAOKE_KO;
             render_priv->state.effect_skip_timing +=
-                    render_priv->state.effect_timing;
+                    (uint32_t) render_priv->state.effect_timing;
             render_priv->state.effect_timing = dtoi32(val * 10);
         } else if (tag("k")) {
             double val = 100;
@@ -812,7 +812,7 @@ char *parse_tags(ASS_Renderer *render_priv, char *p, char *end, double pwr,
                 val = argtod(*args);
             render_priv->state.effect_type = EF_KARAOKE;
             render_priv->state.effect_skip_timing +=
-                    render_priv->state.effect_timing;
+                    (uint32_t) render_priv->state.effect_timing;
             render_priv->state.effect_timing = dtoi32(val * 10);
         } else if (tag("shad")) {
             double val, xval, yval;
@@ -972,7 +972,7 @@ void process_karaoke_effects(ASS_Renderer *render_priv)
 {
     long long tm_current = render_priv->time - render_priv->state.event->Start;
 
-    int timing = 0, skip_timing = 0;
+    int32_t timing = 0, skip_timing = 0;
     Effect effect_type = EF_NONE;
     GlyphInfo *last_boundary = NULL;
     for (int i = 0; i <= render_priv->text_info.length; i++) {
@@ -982,7 +982,7 @@ void process_karaoke_effects(ASS_Renderer *render_priv)
             // break, subsequent text is still part of the same karaoke word,
             // the current word's starting and ending time stay unchanged,
             // but the starting time of the next karaoke word is advanced.
-            skip_timing += render_priv->text_info.glyphs[i].effect_skip_timing;
+            skip_timing += (uint32_t) render_priv->text_info.glyphs[i].effect_skip_timing;
             continue;
         }
 
diff --git a/libass/ass_render.h b/libass/ass_render.h
index 2c5d515..e86219f 100644
--- a/libass/ass_render.h
+++ b/libass/ass_render.h
@@ -104,7 +104,7 @@ typedef struct {
     // during render_and_combine_glyphs: distance in subpixels from the karaoke origin.
     // after render_and_combine_glyphs: screen coordinate in pixels.
     // part of the glyph to the left of it is displayed in a different color.
-    int effect_timing;
+    int32_t effect_timing;
 
     // karaoke origin: screen coordinate of leftmost post-transform control point x in subpixels
     int32_t leftmost_x;
@@ -147,10 +147,10 @@ typedef struct glyph_info {
     ASS_Vector advance;         // 26.6
     ASS_Vector cluster_advance;
     Effect effect_type;
-    int effect_timing;          // time duration of current karaoke word
+    int32_t effect_timing;          // time duration of current karaoke word
     // after process_karaoke_effects: distance in subpixels from the karaoke origin.
     // part of the glyph to the left of it is displayed in a different color.
-    int effect_skip_timing;     // delay after the end of last karaoke word
+    int32_t effect_skip_timing;     // delay after the end of last karaoke word
     int asc, desc;              // font max ascender and descender
     int be;                     // blur edges
     double blur;                // gaussian blur
@@ -252,8 +252,8 @@ typedef struct {
     int clip_drawing_mode;      // 0 = regular clip, 1 = inverse clip
 
     Effect effect_type;
-    int effect_timing;
-    int effect_skip_timing;
+    int32_t effect_timing;
+    int32_t effect_skip_timing;
 
     enum {
         SCROLL_LR,              // left-to-right