[MPlayer-cvslog] r20298 - in trunk: libmpdemux/demux_ogg.c subreader.c

[Ivan Kalvachev]

2006/10/18, reimar <subversion at mplayerhq.hu>:
> Author: reimar
> Date: Wed Oct 18 18:09:59 2006
> New Revision: 20298
>
> Modified:
>    trunk/libmpdemux/demux_ogg.c
>    trunk/subreader.c
>
> Log:
> redone subcp_recode: get rid of static buffer, skip lines that failed to
> convert instead of removing all remaining lines and remove subcp_recode1
> since subcp_recode should now work just as well.
>
>
> Modified: trunk/libmpdemux/demux_ogg.c
> ==============================================================================
> --- trunk/libmpdemux/demux_ogg.c        (original)
> +++ trunk/libmpdemux/demux_ogg.c        Wed Oct 18 18:09:59 2006
> @@ -293,7 +293,7 @@
>    mp_msg(MSGT_DEMUX,MSGL_DBG2,"Ogg sub lines: %d  first: '%s'\n",
>        ogg_sub.lines, ogg_sub.text[0]);
>  #ifdef USE_ICONV
> -  subcp_recode1(&ogg_sub);
> +  subcp_recode(&ogg_sub);
>  #endif
>    vo_sub = &ogg_sub;
>    vo_osd_changed(OSDTYPE_SUBTITLE);
>
> Modified: trunk/subreader.c
> ==============================================================================
> --- trunk/subreader.c   (original)
> +++ trunk/subreader.c   Wed Oct 18 18:09:59 2006
> @@ -1116,69 +1116,35 @@
>         }
>  }
>
> -#define ICBUFFSIZE 512
> -static char icbuffer[ICBUFFSIZE];
> -
> -static subtitle* subcp_recode (subtitle *sub)
> +subtitle* subcp_recode (subtitle *sub)
>  {
>         int l=sub->lines;
>         size_t ileft, oleft;
>         char *op, *ip, *ot;
> +       if(icdsc == (iconv_t)(-1)) return sub;
>
>         while (l){
> -               op = icbuffer;
>                 ip = sub->text[--l];
>                 ileft = strlen(ip);
> -               oleft = ICBUFFSIZE - 1;
> +               oleft = 4 * ileft;
>
> +               if (!(ot = malloc(oleft + 1))){
> +                       mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error allocating mem.\n");
> +                       continue;
> +               }
> +               op = ot;
>                 if (iconv(icdsc, &ip, &ileft,
>                           &op, &oleft) == (size_t)(-1)) {
> -                       mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error recoding line (1).\n");
> -                       l++;
> -                       break;
> -               }
> -               if (!(ot = malloc(op - icbuffer + 1))){
> -                       mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error allocating mem.\n");
> -                       l++;
> -                       break;
> +                       mp_msg(MSGT_SUBREADER,MSGL_WARN,"SUB: error recoding line.\n");
> +                       free(ot);
> +                       continue;
>                 }
>                 *op='\0' ;
> -               strcpy (ot, icbuffer);
>                 free (sub->text[l]);
>                 sub->text[l] = ot;
>         }
> -       if (l){
> -               for (l = sub->lines; l;)
> -                       free (sub->text[--l]);
> -               return ERR;
> -       }
>         return sub;
>  }
> -
> -// for demux_ogg.c:
> -subtitle* subcp_recode1 (subtitle *sub)
> -{
> -  int l=sub->lines;
> -  size_t ileft, oleft;
> -
> -  if(icdsc == (iconv_t)(-1)) return sub;
> -
> -  while (l){
> -     char *ip = icbuffer;
> -     char *op = sub->text[--l];
> -     strlcpy(ip, op, ICBUFFSIZE);
> -     ileft = strlen(ip);
> -     oleft = ICBUFFSIZE - 1;
> -
> -     if (iconv(icdsc, &ip, &ileft,
> -             &op, &oleft) == (size_t)(-1)) {
> -       mp_msg(MSGT_SUBREADER,MSGL_V,"SUB: error recoding line (2).\n");
> -       return sub;
> -     }
> -     *op='\0' ;
> -  }
> -  return sub;
> -}
>  #endif
>
>  #ifdef USE_FRIBIDI


Unfortunately this introduces bug leading to buffer overflow.

On first subtitle the recode function would replace the pre-allocated
buffer (of size OGG_SUB_MAX_LINE, aka  128) with new buffer with size
of  (4*strlen()+1 ).  If next raw subtitle is bigger than that (e.g. 5
times bigger), it would overflow that buffer.