From c946ae4fba7fd2215470991de060ddfb08bcb856 Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 4 Feb 2017 04:02:50 +0200 Subject: Fix decode_font when size % 4 != 0 or data contains illegal bytes When given a byte c, decode_chars expects that 0 <= c - 33 <= 63, i. e. that only the six lowest bits of c - 33 are possibly set. With this assumption, it shifts and adds together multiple c - 33 values. When c > 96, c - 33 has high nonzero bits, which interferes with other shifted terms. c < 33 is even worse: c - 33 is negative (if unsigned char fits in int), and left-shifting negative numbers has undefined behavior. Even before the shift, on common platforms with a two's complement representation of negative integers (or if unsigned char does not fit in int and is promoted to unsigned int), c - 33 has high nonzero bits, which again interfere with other shifted terms. To make matters worse, even perfectly valid encoded data is affected when size % 4 != 0, as decode_font calls decode_chars with '\0', which leads decode_chars to shift and add -33, causing undefined behavior and/or incorrect output. Take our cue from VSFilter and bit-mask c - 33 to keep only the six relevant bits. To ensure that we get the same bits as VSFilter when c < 33 and to avoid the undefined behavior of left-shifting negative numbers, convert the number to unsigned before masking and shifting. While we are at it, rewrite decode_chars entirely to get rid of any GPL code from mkvtoolnix. Related mkvtoolnix bug: https://github.com/mbunkus/mkvtoolnix/issues/1003 Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=516. Also allocate exactly the right amount of memory for the font, because why not. --- libass/ass.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/libass/ass.c b/libass/ass.c index d99498e..cdaf66e 100644 --- a/libass/ass.c +++ b/libass/ass.c @@ -659,24 +659,18 @@ static int process_events_line(ASS_Track *track, char *str) return 0; } -// Copied from mkvtoolnix -static unsigned char *decode_chars(unsigned char c1, unsigned char c2, - unsigned char c3, unsigned char c4, - unsigned char *dst, int cnt) +static unsigned char *decode_chars(const unsigned char *src, + unsigned char *dst, int cnt_in) { - uint32_t value; - unsigned char bytes[3]; - int i; - - value = - ((c1 - 33) << 18) + ((c2 - 33) << 12) + ((c3 - 33) << 6) + (c4 - - 33); - bytes[2] = value & 0xff; - bytes[1] = (value & 0xff00) >> 8; - bytes[0] = (value & 0xff0000) >> 16; - - for (i = 0; i < cnt; ++i) - *dst++ = bytes[i]; + uint32_t value = 0; + for (int i = 0; i < cnt_in; i++) + value |= (uint32_t) ((src[i] - 33u) & 63) << 6 * (3 - i); + + *dst++ = value >> 16; + if (cnt_in >= 3) + *dst++ = value >> 8 & 0xff; + if (cnt_in >= 4) + *dst++ = value & 0xff; return dst; } @@ -696,21 +690,21 @@ static int decode_font(ASS_Track *track) ass_msg(track->library, MSGL_ERR, "Bad encoded data size"); goto error_decode_font; } - buf = malloc(size / 4 * 3 + 2); + buf = malloc(size / 4 * 3 + FFMAX(size % 4 - 1, 0)); if (!buf) goto error_decode_font; q = buf; for (i = 0, p = (unsigned char *) track->parser_priv->fontdata; i < size / 4; i++, p += 4) { - q = decode_chars(p[0], p[1], p[2], p[3], q, 3); + q = decode_chars(p, q, 4); } if (size % 4 == 2) { - q = decode_chars(p[0], p[1], 0, 0, q, 1); + q = decode_chars(p, q, 2); } else if (size % 4 == 3) { - q = decode_chars(p[0], p[1], p[2], 0, q, 2); + q = decode_chars(p, q, 3); } dsize = q - buf; - assert(dsize <= size / 4 * 3 + 2); + assert(dsize == size / 4 * 3 + FFMAX(size % 4 - 1, 0)); if (track->library->extract_fonts) { ass_add_font(track->library, track->parser_priv->fontname, -- cgit v1.2.3