xorl %eax, %eax

MPlayer SAMI Subtitles Parser Buffer Overflow

leave a comment »

This was discovered and reported by Jacques Louw of MWR InfoSecurity and it affects the following software.
– MPlayer SVN before 33471
– SMPlayer 0.6.9 and older
You can read the official security advisory released for this issue here. Moving to the vulnerable code, we can find it in sub/subreader.c source code file in the C function you see below.

static subtitle *sub_read_line_sami(stream_t* st, subtitle *current, int utf16) {
    static char line[LINE_LEN+1];
    static char *s = NULL, *slacktime_s;
    char text[LINE_LEN+1], *p=NULL, *q;
    int state;

    current->lines = current->start = current->end = 0;
    current->alignment = SUB_ALIGNMENT_BOTTOMCENTER;
    state = 0;

    /* read the first line */
    if (!s)
            if (!(s = stream_read_line(st, line, LINE_LEN, utf16))) return 0;

    do {
        switch (state) {
  ...
        case 3: /* get all text until '<' appears */
            if (*s == '\0') break;
            else if (!strncasecmp (s, "<br>", 4)) {
                sami_add_line(current, text, &p);
                s += 4;
            }
            else if ((*s == '{') && !sub_no_text_pp) { state = 5; ++s; continue; }
            else if (*s == '<') { state = 4; }
            else if (!strncasecmp (s, "&nbsp;", 6)) { *p++ = ' '; s += 6; }
            else if (*s == '\t') { *p++ = ' '; s++; }
            else if (*s == '\r' || *s == '\n') { s++; }
            else *p++ = *s++;

            /* skip duplicated space */
            if (p > text + 2) if (*(p-1) == ' ' && *(p-2) == ' ') p--;

            continue;
  ...
    return current;
}

So, basically it will continue appending data to ‘p’ regardless of its size. From the security advisory you can also find the “malicious.smi” file that could be used to trigger this vulnerability. It is the following.

<SAMI>
<BODY>
<SYNC Start=100550>
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
</SAMI>

At last, the patch to fix this vulnerability is the following.

 	case 3: /* get all text until '<' appears */
+	    if (p - text >= LINE_LEN)
+	        sami_add_line(current, text, &p);
 	    if (*s == '\0') break;

As you can see, it checks the line’s length and if there is no space left, it will add a new line by calling sami_add_line() routine.

Written by xorl

October 19, 2011 at 09:04

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s