subreader: fix out of bound write access when parsing .srt

This broke .srt subtitles on gcc-4.8. The breakage was relatively
subtle: it set all hour components to 0, while everything else was
parsed successfully.

But the problem is really that sscanf wrote 1 byte past the sep
variable (or more, for invalid/specially prepared input). The %[..]
format specifier is unbounded. Fix that by letting sscanf drop the
parsed contents with "*", and also make it skip only one input
character by adding "1" (=> "%*1[...").

The out of bound write could easily lead to security issues.

Also, this change makes .srt subtitle parsing slightly more strict.
Strictly speaking this is an unrelated change, but do it anyway. It's
more correct.
This commit is contained in:
wm4 2013-04-30 00:09:31 +02:00
parent 1c96f51e36
commit d98e61ea43
1 changed files with 4 additions and 4 deletions

View File

@ -386,14 +386,14 @@ static subtitle *sub_ass_read_line_subviewer(stream_t *st, subtitle *current,
int a1, a2, a3, a4, b1, b2, b3, b4, j = 0; int a1, a2, a3, a4, b1, b2, b3, b4, j = 0;
while (!current->text[0]) { while (!current->text[0]) {
char line[LINE_LEN + 1], full_line[LINE_LEN + 1], sep; char line[LINE_LEN + 1], full_line[LINE_LEN + 1];
int i; int i;
/* Parse SubRip header */ /* Parse SubRip header */
if (!stream_read_line(st, line, LINE_LEN, utf16)) if (!stream_read_line(st, line, LINE_LEN, utf16))
return NULL; return NULL;
if (sscanf(line, "%d:%d:%d%[,.:]%d --> %d:%d:%d%[,.:]%d", if (sscanf(line, "%d:%d:%d%*1[,.:]%d --> %d:%d:%d%*1[,.:]%d",
&a1, &a2, &a3, &sep, &a4, &b1, &b2, &b3, &sep, &b4) < 10) &a1, &a2, &a3, &a4, &b1, &b2, &b3, &b4) < 8)
continue; continue;
current->start = a1 * 360000 + a2 * 6000 + a3 * 100 + a4 / 10; current->start = a1 * 360000 + a2 * 6000 + a3 * 100 + a4 / 10;
@ -450,7 +450,7 @@ static subtitle *sub_read_line_subviewer(stream_t *st,subtitle *current,
return sub_ass_read_line_subviewer(st, current, args); return sub_ass_read_line_subviewer(st, current, args);
while (!current->text[0]) { while (!current->text[0]) {
if (!stream_read_line (st, line, LINE_LEN, utf16)) return NULL; if (!stream_read_line (st, line, LINE_LEN, utf16)) return NULL;
if ((len=sscanf (line, "%d:%d:%d%[,.:]%d --> %d:%d:%d%[,.:]%d",&a1,&a2,&a3,(char *)&i,&a4,&b1,&b2,&b3,(char *)&i,&b4)) < 10) if ((len=sscanf (line, "%d:%d:%d%*1[,.:]%d --> %d:%d:%d%*1[,.:]%d",&a1,&a2,&a3,&a4,&b1,&b2,&b3,&b4)) < 8)
continue; continue;
current->start = a1*360000+a2*6000+a3*100+a4/10; current->start = a1*360000+a2*6000+a3*100+a4/10;
current->end = b1*360000+b2*6000+b3*100+b4/10; current->end = b1*360000+b2*6000+b3*100+b4/10;