[FFmpeg-devel] [PATCH] avformat/pjsdec: check strcspn values before using them

[Michael Niedermayer]

On Sat, Jan 11, 2014 at 12:14:52PM +0100, Clément Bœsch wrote:
> On Sat, Jan 11, 2014 at 02:44:04AM +0100, Michael Niedermayer wrote:
> [...]
> > > >          pts_start = read_ts(&p, &duration);
> > > >          if (pts_start != AV_NOPTS_VALUE) {
> > > >              AVPacket *sub;
> > > >  
> > > > -            p[strcspn(p, "\"")] = 0;
> > > > +            idx = strcspn(p, "\"");
> > > > +            if (!p[idx]) {
> > > > +                av_log(s, AV_LOG_ERROR, "missing \"\n");
> > > > +                return AVERROR_INVALIDDATA;
> > > > +            }
> > > > +
> > > > +            p[idx] = 0;
> > > >              sub = ff_subtitles_queue_insert(&pjs->q, p, strlen(p), 0);
> > > >              if (!sub)
> > > >                  return AVERROR(ENOMEM);
> > > 
> > > The use of strcspn() as such is fine (and we use it everywhere). I'd suggest
> > 
> > it is but i think the code would benefit from some checks and
> > warnings or errors over just keeping the pointer within the array
> > and producing "some" output for any arbitrary random input.
> > 
> 
> I'm not really against, but if you plan to do that change, please do it in
> all the other demuxers, it's a recurring pattern which is meant to be
> reproduced over and over again if not fixed everywhere.

can these reoccurances be factorized into a common function ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140111/c5baf1fa/attachment.asc>