Ticket #2139: subreader-fix-srt-parsing.patch

File subreader-fix-srt-parsing.patch, 2.6 KB (added by evangelos@…, 13 years ago)

Fix from the mpv-player project with tweaked variable names in the first chunk

  • sub/subreader.c 

    From d98e61ea438db66323734ad1b6bea66411a3c97b Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Tue, 30 Apr 2013 00:09:31 +0200
Subject: [PATCH] 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.
---
 sub/subreader.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

 (foutrelis: adjusted variable names in first hunk to apply to mplayer)

diff --git a/sub/subreader.c b/sub/subreader.c
index 23da4c7..0f1b6c9 100644
    a b static subtitle *sub_ass_read_line_subviewer(stream_t *st, subtitle *current,  
    386386    int h1, m1, s1, ms1, h2, m2, s2, ms2, j = 0;
    387387
    388388    while (!current->text[0]) {
    389         char line[LINE_LEN + 1], full_line[LINE_LEN + 1], sep;
     389        char line[LINE_LEN + 1], full_line[LINE_LEN + 1];
    390390        int i;
    391391
    392392        /* Parse SubRip header */
    393393        if (!stream_read_line(st, line, LINE_LEN, utf16))
    394394            return NULL;
    395         if (sscanf(line, "%d:%d:%d%[,.:]%d --> %d:%d:%d%[,.:]%d",
    396                      &h1, &m1, &s1, &sep, &ms1, &h2, &m2, &s2, &sep, &ms2) < 10)
     395        if (sscanf(line, "%d:%d:%d%*1[,.:]%d --> %d:%d:%d%*1[,.:]%d",
     396                     &h1, &m1, &s1, &ms1, &h2, &m2, &s2, &ms2) < 8)
    397397            continue;
    398398
    399399        current->start = a1 * 360000 + a2 * 6000 + a3 * 100 + a4 / 10;
    … … static subtitle *sub_read_line_subviewer(stream_t *st,subtitle *current,  
    450450        return sub_ass_read_line_subviewer(st, current, args);
    451451    while (!current->text[0]) {
    452452        if (!stream_read_line (st, line, LINE_LEN, utf16)) return NULL;
    453         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)
     453        if ((len=sscanf (line, "%d:%d:%d%*1[,.:]%d --> %d:%d:%d%*1[,.:]%d",&a1,&a2,&a3,&a4,&b1,&b2,&b3,&b4)) < 8)
    454454            continue;
    455455        current->start = a1*360000+a2*6000+a3*100+a4/10;
    456456        current->end   = b1*360000+b2*6000+b3*100+b4/10;