diff options
author | Oleg Oshmyan <chortos@inbox.lv> | 2017-02-04 04:02:50 +0200 |
---|---|---|
committer | Oleg Oshmyan <chortos@inbox.lv> | 2017-02-14 19:43:41 +0200 |
commit | c946ae4fba7fd2215470991de060ddfb08bcb856 (patch) | |
tree | 1a718a1785024cac0db641a3b312e94ef697905f | |
parent | 92c359721218bbdd2949ea29e13b1cf655f88920 (diff) | |
download | libass-c946ae4fba7fd2215470991de060ddfb08bcb856.tar.bz2 libass-c946ae4fba7fd2215470991de060ddfb08bcb856.tar.xz |
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.
-rw-r--r-- | libass/ass.c | 38 |
1 files changed, 16 insertions, 22 deletions
diff --git a/libass/ass.c b/libass/ass.c index d99498ec..cdaf66ed 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, |