summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOleg Oshmyan <chortos@inbox.lv>2017-02-04 04:02:50 +0200
committerOleg Oshmyan <chortos@inbox.lv>2017-02-14 19:43:41 +0200
commitc946ae4fba7fd2215470991de060ddfb08bcb856 (patch)
tree1a718a1785024cac0db641a3b312e94ef697905f
parent92c359721218bbdd2949ea29e13b1cf655f88920 (diff)
downloadlibass-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.c38
1 files 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,