summaryrefslogtreecommitdiffstats
path: root/sub
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2013-04-30 00:09:31 +0200
committerwm4 <wm4@nowhere>2013-04-30 00:14:26 +0200
commitd98e61ea438db66323734ad1b6bea66411a3c97b (patch)
treee26e4e12ceb9c305bc45240989dc2ed0f4994594 /sub
parent1c96f51e36593d43843e8bd41cee49a2bee5e4dc (diff)
downloadmpv-d98e61ea438db66323734ad1b6bea66411a3c97b.tar.bz2
mpv-d98e61ea438db66323734ad1b6bea66411a3c97b.tar.xz
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.
Diffstat (limited to 'sub')
-rw-r--r--sub/subreader.c8
1 files changed, 4 insertions, 4 deletions
diff --git a/sub/subreader.c b/sub/subreader.c
index 23da4c79e5..0f1b6c9bbd 100644
--- a/sub/subreader.c
+++ b/sub/subreader.c
@@ -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;
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;
/* Parse SubRip header */
if (!stream_read_line(st, line, LINE_LEN, utf16))
return NULL;
- if (sscanf(line, "%d:%d:%d%[,.:]%d --> %d:%d:%d%[,.:]%d",
- &a1, &a2, &a3, &sep, &a4, &b1, &b2, &b3, &sep, &b4) < 10)
+ if (sscanf(line, "%d:%d:%d%*1[,.:]%d --> %d:%d:%d%*1[,.:]%d",
+ &a1, &a2, &a3, &a4, &b1, &b2, &b3, &b4) < 8)
continue;
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);
while (!current->text[0]) {
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;
current->start = a1*360000+a2*6000+a3*100+a4/10;
current->end = b1*360000+b2*6000+b3*100+b4/10;