From 3ebd1945b44327450fec3fdd164f591921c3f281 Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Wed, 28 Dec 2016 21:14:21 +0200 Subject: Fix buffer overread in parse_tag when end points to a space When parse_tag is invoked recursively to handle the animated tags inside a \t tag, the `end` argument is taken from the `end` field of a struct arg in the enclosing parse_tag. When struct arg is filled by push_arg, this field is always right-trimmed using rskip_spaces. Ultimately, the inner parse_tag invokation sees its `end` argument point not to the ')' or '}' of the \t as it expects but rather to the spaces preceding the ')' or '}'. At this point, when parse_tag calls skip_spaces, which is ignorant of the end pointer, it happily skips over the spaces preceding the ')', moving the pointer past `end`. Subsequent `pointer != end` comparisons in parse_tag fail (as in fact `pointer > end`), and parse_tag thinks it is still inside the substring to be parsed. This is harmless in many cases, but given either of the following inputs, parse_tag reads past the end of the actual buffer that stores the string: {\t(\ } {\t(\ )(} After this commit, parse_tag knows that `end` can point to a sequence of spaces and avoids calling skip_spaces on `end`, thus avoiding the overread. Discovered by OSS-Fuzz. Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=194. --- libass/ass_parse.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libass/ass_parse.c b/libass/ass_parse.c index 5698669..0a25c64 100644 --- a/libass/ass_parse.c +++ b/libass/ass_parse.c @@ -262,7 +262,8 @@ static int parse_vector_clip(ASS_Renderer *render_priv, /** * \brief Parse style override tag. * \param p string to parse - * \param end end of string to parse, which must be '}' or ')' + * \param end end of string to parse, which must be '}', ')', or the first + * of a number of spaces immediately preceding '}' or ')' * \param pwr multiplier for some tag effects (comes from \t tags) */ char *parse_tag(ASS_Renderer *render_priv, char *p, char *end, double pwr) @@ -272,7 +273,8 @@ char *parse_tag(ASS_Renderer *render_priv, char *p, char *end, double pwr) if (*p != '\\') return p; ++p; - skip_spaces(&p); + if (p != end) + skip_spaces(&p); char *q = p; while (*q != '(' && *q != '\\' && q != end) @@ -293,7 +295,8 @@ char *parse_tag(ASS_Renderer *render_priv, char *p, char *end, double pwr) if (*q == '(') { ++q; while (1) { - skip_spaces(&q); + if (q != end) + skip_spaces(&q); // Split on commas. If there is a backslash, ignore any // commas following it and lump everything starting from -- cgit v1.2.3