diff options
author | Oneric <oneric@oneric.stub> | 2022-04-16 00:22:02 +0200 |
---|---|---|
committer | Oneric <oneric@oneric.stub> | 2022-04-26 21:38:12 +0200 |
commit | d1140ac30240c0490da861d6ed8c76cc931584db (patch) | |
tree | 27242972add9320dafb5da7f84835e5599d5902d | |
parent | 85c8c6d7be14cc2602b92ec715834b9c1069a325 (diff) | |
download | libass-d1140ac30240c0490da861d6ed8c76cc931584db.tar.bz2 libass-d1140ac30240c0490da861d6ed8c76cc931584db.tar.xz |
render: avoid UB on left shifts
The two shifts could be used with too large or negative shift operands
which is undefined behaviour. To avoid UB but keep VSFilter
compatibility, simulate the practical effects of such shifts on x86
Windows instead of directly using a simple shift.
The simulated behaviour matches MSYS2's gcc. Clang, gcc and tcc were
observed to exhibit different behaviour on Linux, iff the operand was
a compile-time constant (they would use an arithmetic right shift
instead). Ideally this should be checked against MSVC, but it seems
unlikely to deviate since the related x86 instruction SHL/SAL
is documented to mask the upper three bits of the count operand.
The > 0 check is not present in VSFilter, but we'd like to avoid
negative scales and the only possible negative value is INT32_MIN
i.e. -2^31. Luckily using zero instead of the (font_scale / INT32_MIN)
won't make much difference here.
In guliverkli2 parsed drawing coordinates are also int32_t in canvas
space and only later multiplied by (64 * m_scale{x,y}) where m_scale
combines canvas and drawing scale and is a double. The result is then
cast back to int32_t, meaning coordinates absolutley larger than or
equal to min(2^(25 + draw_scale_exp) / canvas_scale{x,y}, INT32_MAX)
will be turned into INT32_MIN. For the \p32 case this means any drawing
without overflow will be at most 2×2 canvas units large. If only some
coordinates overflow the result can cover more.
In xy-VSFilter drawing coordinates are floating point numbers and parsed
as 64-bit IEEE floats, then multiplied by 64 and cast to int32_t. Here
a truncation already happens before the scaling values are applied,
so any coordinate larger than 2^25 turn into INT32_MIN during initial
parsing. For the \p32 this means a non-overflowing drawing is at most
1/32×1/32 canvas units large.
As a consequence of this parsing difference, drawing coordinates larger
than 2^25 are non-portable showing different behaviour in guliverkli2-
and xy-VSFilter. Thus treating \p32 as an effective \fscx0 seems
unproblematic, since even when scaling the default height of 288
up to a UHD frame size, 1/32×1/32 canvas units don't extend beyond 1/4
pixel, which unless stacked won't be too noticeable.
The remaining left shifts in ass_render.c, should all be safe as they
shift a constant 1 and use macros defined by us as the shift operands
and blur_radius which is limited to 100.
Found by AFL++ and UBSAN.
Samples
assert-fail+shift9999999_fuzzer_w3:id:000159,sig:04,src:000968+000719,time:452375,execs:57769,op:splice,rep:16
shift125_id:000001,sig:04,src:000008+000014,time:526595,execs:18418,op:splice,rep:8
shift-neg_id:000000,sig:04,src:000008+000014,time:466566,execs:18013,op:splice,rep:16
-rw-r--r-- | libass/ass_render.c | 6 | ||||
-rw-r--r-- | libass/ass_utils.h | 6 |
2 files changed, 10 insertions, 2 deletions
diff --git a/libass/ass_render.c b/libass/ass_render.c index cf22bec..b0accb7 100644 --- a/libass/ass_render.c +++ b/libass/ass_render.c @@ -662,7 +662,8 @@ static void blend_vector_clip(ASS_Renderer *render_priv, ASS_Image *head) ol_key.u.drawing.text = render_priv->state.clip_drawing_text; double m[3][3] = {{0}}; - double w = render_priv->font_scale / (1 << (render_priv->state.clip_drawing_scale - 1)); + int32_t scale_base = lshiftwrapi(1, render_priv->state.clip_drawing_scale - 1); + double w = scale_base > 0 ? (render_priv->font_scale / scale_base) : 0; m[0][0] = render_priv->font_scale_x * w; m[1][1] = w; m[2][2] = 1; @@ -1105,7 +1106,8 @@ get_outline_glyph(ASS_Renderer *priv, GlyphInfo *info) return; } - double w = priv->font_scale / (1 << (info->drawing_scale - 1)); + int32_t scale_base = lshiftwrapi(1, info->drawing_scale - 1); + double w = scale_base > 0 ? (priv->font_scale / scale_base) : 0; scale.x = info->scale_x * w; scale.y = info->scale_y * w; desc = 64 * info->drawing_pbo; diff --git a/libass/ass_utils.h b/libass/ass_utils.h index dbe556c..3e239b5 100644 --- a/libass/ass_utils.h +++ b/libass/ass_utils.h @@ -182,6 +182,12 @@ static inline int double_to_d22(double x) return lrint(x * 0x400000); } +static inline int32_t lshiftwrapi(int32_t i, int32_t shift) +{ + // '0u +' to avoid automatic integer promotion causing UB + return (0u + (uint32_t)i) << (shift & 31); +} + static inline int mystrtod(char **p, double *res) { char *start = *p; |