On Fri, 9 Jun 2017 19:39:36 +0200
Michael Niedermayer <mich...@niedermayer.cc> wrote:
> On Fri, Jun 09, 2017 at 04:32:41PM +0200, wm4 wrote:
> > On Thu, 8 Jun 2017 23:53:55 +0200
> > Michael Niedermayer <mich...@niedermayer.cc> wrote:
> >
> > > Fixes Timeout
> > > Fixes: 2127/clusterfuzz-testcase-minimized-6595787859427328
> > >
> > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> > > ---
> > > libavcodec/htmlsubtitles.c | 12 ++++++++++--
> > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
> > > index 16295daa0c..ba4f269b3f 100644
> > > --- a/libavcodec/htmlsubtitles.c
> > > +++ b/libavcodec/htmlsubtitles.c
> > > @@ -56,6 +56,7 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst,
> > > const char *in)
> > > char *param, buffer[128], tmp[128];
> > > int len, tag_close, sptr = 1, line_start = 1, an = 0, end = 0;
> > > SrtStack stack[16];
> > > + const char *next_closep = NULL;
> > >
> > > stack[0].tag[0] = 0;
> > > strcpy(stack[0].param[PARAM_SIZE], "{\\fs}");
> > > @@ -83,8 +84,15 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst,
> > > const char *in)
> > > and all microdvd like styles such as {Y:xxx} */
> > > len = 0;
> > > an += sscanf(in, "{\\an%*1u}%n", &len) >= 0 && len > 0;
> > > - if ((an != 1 && (len = 0, sscanf(in, "{\\%*[^}]}%n", &len)
> > > >= 0 && len > 0)) ||
> > > - (len = 0, sscanf(in, "{%*1[CcFfoPSsYy]:%*[^}]}%n", &len)
> > > >= 0 && len > 0)) {
> > > +
> > > + if(!next_closep || next_closep <= in) {
> > > + next_closep = strchr(in+1, '}');
> > > + if (!next_closep)
> > > + next_closep = in + strlen(in);
> > > + }
> > > +
> > > + if (*next_closep == '}' && (an != 1 && (len = 0, sscanf(in,
> > > "{\\%*[^}]}%n", &len) >= 0 && len > 0)) ||
> > > + *next_closep == '}' && (len = 0, sscanf(in,
> > > "{%*1[CcFfoPSsYy]:%*[^}]}%n", &len) >= 0 && len > 0)) {
> > > in += len - 1;
> > > } else
> > > av_bprint_chars(dst, *in, 1);
> >
> > I'm not convinced that bad performance with an obscure fuzzed sample is
> > worth the complexity increase here.
>
> If we want to make ff_htmlmarkup_to_ass() not go into near infinite
> loops with crafted data then this or a equivalent change is likely
> needed.
Maybe if someone starts abusing this case.
>
> >
> > I'd rather ask, why the heck is it using sscanf in the first place?
>
> I suspect the use of scanf is fewer lines of code than the alternative
> but iam not the author of this, i dont know for sure why it was written
> as it was.
I'd probably prefer explicit code here (as long as it's not typical
"tricky C string processing" style), but it's hard to tell.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
