Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Fix false positive in infinite loop detector

[Michael Niedermayer]

On Mon, Oct 27, 2014 at 04:52:26PM +0100, tomas.har...@codemill.se wrote:
> On 2014-10-27 16:27, Michael Niedermayer wrote:
> >Fixes Ticket4040
> >
> >Signed-off-by: Michael Niedermayer 
> >---
> > libavformat/mxfdec.c |   11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> >diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> >index b01dd0c..a1abc34 100644
> >--- a/libavformat/mxfdec.c
> >+++ b/libavformat/mxfdec.c
> >@@ -2211,6 +2211,13 @@ end:
> > avio_seek(s->pb, mxf->run_in, SEEK_SET);
> > }
> >
> >+static uint64_t loop_detection_state(AVFormatContext *s)
> >+{
> >+MXFContext *mxf = s->priv_data;
> >+
> >+return avio_tell(s->pb) + 0xA987654321*!mxf->current_partition;
> >+}
> >+
> 
> What the hell? Just use a flag or something, or
> mxf->parsing_backward (preferably)

i tried parsing_backward before posting this, sadly it didnt help
Ticket4040, at least not in mxf->current_partition place above or i
made some dumb mistake

"mxf->current_partition != NULL" was used as this was part of the
condition that made the 2 occurances of the same position diverge

The patch surely is ugly, i dont like it at all either ...


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: Fix false positive in infinite loop detector

[tomas . hardin]

On 2014-10-27 16:27, Michael Niedermayer wrote:

Fixes Ticket4040

Signed-off-by: Michael Niedermayer 
---
 libavformat/mxfdec.c |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index b01dd0c..a1abc34 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2211,6 +2211,13 @@ end:
 avio_seek(s->pb, mxf->run_in, SEEK_SET);
 }

+static uint64_t loop_detection_state(AVFormatContext *s)
+{
+MXFContext *mxf = s->priv_data;
+
+return avio_tell(s->pb) + 0xA987654321*!mxf->current_partition;
+}
+


What the hell? Just use a flag or something, or mxf->parsing_backward 
(preferably)



 static int mxf_read_header(AVFormatContext *s)
 {
 MXFContext *mxf = s->priv_data;
@@ -2235,12 +2242,12 @@ static int mxf_read_header(AVFormatContext *s)

 while (!avio_feof(s->pb)) {
 const MXFMetadataReadTableEntry *metadata;
-if (avio_tell(s->pb) == last_pos) {
+if (loop_detection_state(s) == last_pos) {
 av_log(mxf->fc, AV_LOG_ERROR, "MXF structure loop 
detected\n");

 return AVERROR_INVALIDDATA;
 }
 if ((1ULL<<61) % last_pos_index++ == 0)


This looks extremely dubious, but I see 1c010fd03 was a stop gap to fix 
a an issue discovered by fuzzing. Why didn't anyone poke my on IRC about 
it?
I have furniture to move today, after that I might have some time to 
develop an non-awful fix.


/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mxfdec: Fix false positive in infinite loop detector

[Michael Niedermayer]

Fixes Ticket4040

Signed-off-by: Michael Niedermayer 
---
 libavformat/mxfdec.c |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index b01dd0c..a1abc34 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2211,6 +2211,13 @@ end:
 avio_seek(s->pb, mxf->run_in, SEEK_SET);
 }
 
+static uint64_t loop_detection_state(AVFormatContext *s)
+{
+MXFContext *mxf = s->priv_data;
+
+return avio_tell(s->pb) + 0xA987654321*!mxf->current_partition;
+}
+
 static int mxf_read_header(AVFormatContext *s)
 {
 MXFContext *mxf = s->priv_data;
@@ -2235,12 +2242,12 @@ static int mxf_read_header(AVFormatContext *s)
 
 while (!avio_feof(s->pb)) {
 const MXFMetadataReadTableEntry *metadata;
-if (avio_tell(s->pb) == last_pos) {
+if (loop_detection_state(s) == last_pos) {
 av_log(mxf->fc, AV_LOG_ERROR, "MXF structure loop detected\n");
 return AVERROR_INVALIDDATA;
 }
 if ((1ULL<<61) % last_pos_index++ == 0)
-last_pos = avio_tell(s->pb);
+last_pos = loop_detection_state(s);
 if (klv_read_packet(&klv, s->pb) < 0) {
 /* EOF - seek to previous partition or stop */
 if(mxf_parse_handle_partition_or_eof(mxf) <= 0)
-- 
1.7.9.5

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel