diff options
| author | Oneric <oneric@oneric.stub> | 2022-09-28 00:05:50 +0200 |
|---|---|---|
| committer | Oneric <oneric@oneric.stub> | 2022-09-29 21:34:35 +0200 |
| commit | abd7cd51d86d299b72d5827332352bfdbeb359db (patch) | |
| tree | f07153c8aeef7a7a7b76a2951657ced322b9ca51 /libass/ass_parse.c | |
| parent | 486fb4a871b50bd2a7e416266ed3e24cae2d3d2b (diff) | |
| download | libass-abd7cd51d86d299b72d5827332352bfdbeb359db.tar.bz2 libass-abd7cd51d86d299b72d5827332352bfdbeb359db.tar.xz | |
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
Diffstat (limited to 'libass/ass_parse.c')
| -rw-r--r-- | libass/ass_parse.c | 10 |
1 files changed, 5 insertions, 5 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; } |