summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOneric <oneric@oneric.stub>2022-04-16 00:22:02 +0200
committerOneric <oneric@oneric.stub>2022-04-26 21:38:12 +0200
commitd1140ac30240c0490da861d6ed8c76cc931584db (patch)
tree27242972add9320dafb5da7f84835e5599d5902d
parent85c8c6d7be14cc2602b92ec715834b9c1069a325 (diff)
downloadlibass-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.c6
-rw-r--r--libass/ass_utils.h6
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;