[FFmpeg-devel] [PATCH 2/3] nutdec: stop skipping bytes at EOF

[Michael Niedermayer]

On Wed, May 20, 2015 at 04:35:10PM +0200, Andreas Cadhalpun wrote:
> On 20.05.2015 03:15, Michael Niedermayer wrote:
> > On Wed, May 20, 2015 at 12:49:55AM +0200, Andreas Cadhalpun wrote:
> >> This can unnecessarily waste a lot of time.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavformat/nutdec.c | 8 +++++---
> >>  1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> >> index a75587f..e979ee6 100644
> >> --- a/libavformat/nutdec.c
> >> +++ b/libavformat/nutdec.c
> >> @@ -47,6 +47,8 @@ static int get_str(AVIOContext *bc, char *string, unsigned int maxlen)
> >>      while (len > maxlen) {
> >>          avio_r8(bc);
> >>          len--;
> >> +        if (bc->eof_reached)
> >> +            len = maxlen;
> >>      }
> > 
> > maybe this would be better as avio_skip()
> > but ok either way
> 
> I prefer to avoid the additional complexity of avio_skip (see below...).
> 
> >>      if (maxlen)
> >> @@ -211,7 +213,7 @@ static int skip_reserved(AVIOContext *bc, int64_t pos)
> >>          avio_seek(bc, pos, SEEK_CUR);
> >>          return AVERROR_INVALIDDATA;
> >>      } else {
> >> -        while (pos--)
> >> +        while (pos-- && !bc->eof_reached)
> >>              avio_r8(bc);
> >>          return 0;
> >>      }
> >> @@ -291,7 +293,7 @@ static int decode_main_header(NUTContext *nut)
> >>          if (tmp_fields > 7)
> >>              tmp_head_idx = ffio_read_varlen(bc);
> >>  
> >> -        while (tmp_fields-- > 8)
> >> +        while (tmp_fields-- > 8 && !bc->eof_reached)
> >>              ffio_read_varlen(bc);
> >>  
> >>          if (count <= 0 || count > 256 - (i <= 'N') - i) {
> >> @@ -990,7 +992,7 @@ static int decode_frame_header(NUTContext *nut, int64_t *pts, int *stream_id,
> >>          *header_idx = ffio_read_varlen(bc);
> >>      if (flags & FLAG_RESERVED)
> >>          reserved_count = ffio_read_varlen(bc);
> >> -    for (i = 0; i < reserved_count; i++)
> >> +    for (i = 0; i < reserved_count && !bc->eof_reached; i++)
> >>          ffio_read_varlen(bc);
> > 
> > these should return an error in the eof case
> 
> OK, patch updated.
> 
> > the first of the 3 could use a seek/skip as well possibly
> 
> I tried this, but it caused weird crashes in av_crc, so I reverted that.
> 

> > also if you want some of these things could also be limited by te
> > packet end from get_packetheader and not just EOF
> 
> I'm not sure this would help much, because get_packetheader reads the
> size from the file, so it could be an arbitrary value as well. 

yes for a mallicious file but one that is just damaged
could recover quickly and contiue playback if the header size was
used while otherwise it might read till EOF rendering it unwatchable
but thats orthogonal to the patch 

patch LGTM

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150520/6b3f8441/attachment.asc>