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.

diff --git a/libass/ass_parse.c b/libass/ass_parse.c
index 569866976b93968c8ce0244c611d2ceef490522a..0a25c64a953536444079d6e1ce3ffbeed76dc14d 100644 (file)
--- 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