Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-27 Thread Andreas Cadhalpun 
On 27.06.2015 18:02, Michael Niedermayer wrote:
 On Sat, Jun 27, 2015 at 05:53:26PM +0200, Andreas Cadhalpun wrote:
  nutdec.c |3 +++
  1 file changed, 3 insertions(+)
 4e07b069348ca9b9d65b7850291448201c4d81f6  
 0001-nutdec-check-maxpos-in-read_sm_data-before-returning.patch
 From 4e10305531d162fff2a7daac49cc046c771909a9 Mon Sep 17 00:00:00 2001
 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 Date: Sat, 27 Jun 2015 17:50:56 +0200
 Subject: [PATCH] nutdec: check maxpos in read_sm_data before returning 
 success
 
 LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-27 Thread Andreas Cadhalpun 
On 27.06.2015 02:31, Michael Niedermayer wrote:
 On Fri, Jun 26, 2015 at 07:28:36PM +0200, Andreas Cadhalpun wrote:
 On 26.06.2015 01:36, Michael Niedermayer wrote:
 On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
 Otherwise sm_size can be larger than size, which results in a negative
 packet size.

 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  libavformat/nutdec.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)




 diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
 index 13fb399..43bd27b 100644
 --- a/libavformat/nutdec.c
 +++ b/libavformat/nutdec.c
 @@ -888,7 +888,7 @@ fail:
  
  static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket 
 *pkt, int is_meta, int64_t maxpos)
  {
 -int count = ffio_read_varlen(bc);
 +int count;
  int skip_start = 0;
  int skip_end = 0;
  int channels = 0;
 @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, 
 AVIOContext *bc, AVPacket *pkt, int
  int height = 0;
  int i, ret;
  
 +if (avio_tell(bc) = maxpos)
 +return AVERROR_INVALIDDATA;
 +
 +count = ffio_read_varlen(bc);

 ffio_read_varlen() could move the position beyond maxpos yet return
 0 so the loop with teh checks inside is skiped

 That is exactly the problem, because then sm_size can be larger than size.
 An alternative would be to directly check for that, like in attached patch.
 
 wouldnt checking after the loop im read_sm_data() before returning
 success be more robust ?
 It would exit sooner if the problem occurs in the first call
 and avoid potential integer overflows

OK, new patch attached.

 but iam fine with any solution that works

Me too.

Best regards,
Andreas

From 4e10305531d162fff2a7daac49cc046c771909a9 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Sat, 27 Jun 2015 17:50:56 +0200
Subject: [PATCH] nutdec: check maxpos in read_sm_data before returning success

Otherwise sm_size can be larger than size, which results in a negative
packet size.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavformat/nutdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 13fb399..606deaa 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -1005,6 +1005,9 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
 AV_WL32(dst+4, skip_end);
 }
 
+if (avio_tell(bc) = maxpos)
+return AVERROR_INVALIDDATA;
+
 return 0;
 }
 
-- 
2.1.4

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

Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-27 Thread Michael Niedermayer 
On Sat, Jun 27, 2015 at 05:53:26PM +0200, Andreas Cadhalpun wrote:
 On 27.06.2015 02:31, Michael Niedermayer wrote:
  On Fri, Jun 26, 2015 at 07:28:36PM +0200, Andreas Cadhalpun wrote:
  On 26.06.2015 01:36, Michael Niedermayer wrote:
  On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
  Otherwise sm_size can be larger than size, which results in a negative
  packet size.
 
  Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
  ---
   libavformat/nutdec.c | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)
 
 
 
 
  diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
  index 13fb399..43bd27b 100644
  --- a/libavformat/nutdec.c
  +++ b/libavformat/nutdec.c
  @@ -888,7 +888,7 @@ fail:
   
   static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket 
  *pkt, int is_meta, int64_t maxpos)
   {
  -int count = ffio_read_varlen(bc);
  +int count;
   int skip_start = 0;
   int skip_end = 0;
   int channels = 0;
  @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, 
  AVIOContext *bc, AVPacket *pkt, int
   int height = 0;
   int i, ret;
   
  +if (avio_tell(bc) = maxpos)
  +return AVERROR_INVALIDDATA;
  +
  +count = ffio_read_varlen(bc);
 
  ffio_read_varlen() could move the position beyond maxpos yet return
  0 so the loop with teh checks inside is skiped
 
  That is exactly the problem, because then sm_size can be larger than size.
  An alternative would be to directly check for that, like in attached patch.
  
  wouldnt checking after the loop im read_sm_data() before returning
  success be more robust ?
  It would exit sooner if the problem occurs in the first call
  and avoid potential integer overflows
 
 OK, new patch attached.
 
  but iam fine with any solution that works
 
 Me too.
 
 Best regards,
 Andreas
 

  nutdec.c |3 +++
  1 file changed, 3 insertions(+)
 4e07b069348ca9b9d65b7850291448201c4d81f6  
 0001-nutdec-check-maxpos-in-read_sm_data-before-returning.patch
 From 4e10305531d162fff2a7daac49cc046c771909a9 Mon Sep 17 00:00:00 2001
 From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 Date: Sat, 27 Jun 2015 17:50:56 +0200
 Subject: [PATCH] nutdec: check maxpos in read_sm_data before returning success

LGTM

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.


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

Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-26 Thread Andreas Cadhalpun 
On 26.06.2015 01:36, Michael Niedermayer wrote:
 On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
 Otherwise sm_size can be larger than size, which results in a negative
 packet size.

 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  libavformat/nutdec.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 
 

 diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
 index 13fb399..43bd27b 100644
 --- a/libavformat/nutdec.c
 +++ b/libavformat/nutdec.c
 @@ -888,7 +888,7 @@ fail:
  
  static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, 
 int is_meta, int64_t maxpos)
  {
 -int count = ffio_read_varlen(bc);
 +int count;
  int skip_start = 0;
  int skip_end = 0;
  int channels = 0;
 @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, AVIOContext 
 *bc, AVPacket *pkt, int
  int height = 0;
  int i, ret;
  
 +if (avio_tell(bc) = maxpos)
 +return AVERROR_INVALIDDATA;
 +
 +count = ffio_read_varlen(bc);
 
 ffio_read_varlen() could move the position beyond maxpos yet return
 0 so the loop with teh checks inside is skiped

That is exactly the problem, because then sm_size can be larger than size.
An alternative would be to directly check for that, like in attached patch.

Best regards,
Andreas
From 25322c14b9ca46cc1c841dfdcc37ee42d16ea639 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Fri, 26 Jun 2015 19:25:05 +0200
Subject: [PATCH] nutdec: ensure non-negative packet size

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavformat/nutdec.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 13fb399..3d6fb64 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -1136,6 +1136,12 @@ static int decode_frame(NUTContext *nut, AVPacket *pkt, int frame_code)
 goto fail;
 }
 sm_size = avio_tell(bc) - pkt-pos;
+if (size  sm_size) {
+av_log(s, AV_LOG_ERROR, size %d smaller than sm_size %d\n,
+   size, sm_size);
+ret = AVERROR_INVALIDDATA;
+goto fail;
+}
 size  -= sm_size;
 pkt-size -= sm_size;
 }
-- 
2.1.4

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

Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-26 Thread Michael Niedermayer 
On Fri, Jun 26, 2015 at 07:28:36PM +0200, Andreas Cadhalpun wrote:
 On 26.06.2015 01:36, Michael Niedermayer wrote:
  On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
  Otherwise sm_size can be larger than size, which results in a negative
  packet size.
 
  Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
  ---
   libavformat/nutdec.c | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)
  
  
  
 
  diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
  index 13fb399..43bd27b 100644
  --- a/libavformat/nutdec.c
  +++ b/libavformat/nutdec.c
  @@ -888,7 +888,7 @@ fail:
   
   static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket 
  *pkt, int is_meta, int64_t maxpos)
   {
  -int count = ffio_read_varlen(bc);
  +int count;
   int skip_start = 0;
   int skip_end = 0;
   int channels = 0;
  @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, 
  AVIOContext *bc, AVPacket *pkt, int
   int height = 0;
   int i, ret;
   
  +if (avio_tell(bc) = maxpos)
  +return AVERROR_INVALIDDATA;
  +
  +count = ffio_read_varlen(bc);
  
  ffio_read_varlen() could move the position beyond maxpos yet return
  0 so the loop with teh checks inside is skiped
 
 That is exactly the problem, because then sm_size can be larger than size.
 An alternative would be to directly check for that, like in attached patch.

wouldnt checking after the loop im read_sm_data() before returning
success be more robust ?
It would exit sooner if the problem occurs in the first call
and avoid potential integer overflows

but iam fine with any solution that works

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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

[FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-25 Thread Andreas Cadhalpun 
Otherwise sm_size can be larger than size, which results in a negative
packet size.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavformat/nutdec.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 13fb399..43bd27b 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -888,7 +888,7 @@ fail:
 
 static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, 
int is_meta, int64_t maxpos)
 {
-int count = ffio_read_varlen(bc);
+int count;
 int skip_start = 0;
 int skip_end = 0;
 int channels = 0;
@@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, AVIOContext 
*bc, AVPacket *pkt, int
 int height = 0;
 int i, ret;
 
+if (avio_tell(bc) = maxpos)
+return AVERROR_INVALIDDATA;
+
+count = ffio_read_varlen(bc);
+
 for (i=0; icount; i++) {
 uint8_t name[256], str_value[256], type_str[256];
 int value;
-- 
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

2015-06-25 Thread Michael Niedermayer 
On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
 Otherwise sm_size can be larger than size, which results in a negative
 packet size.
 
 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  libavformat/nutdec.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)



 
 diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
 index 13fb399..43bd27b 100644
 --- a/libavformat/nutdec.c
 +++ b/libavformat/nutdec.c
 @@ -888,7 +888,7 @@ fail:
  
  static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, 
 int is_meta, int64_t maxpos)
  {
 -int count = ffio_read_varlen(bc);
 +int count;
  int skip_start = 0;
  int skip_end = 0;
  int channels = 0;
 @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, AVIOContext 
 *bc, AVPacket *pkt, int
  int height = 0;
  int i, ret;
  
 +if (avio_tell(bc) = maxpos)
 +return AVERROR_INVALIDDATA;
 +
 +count = ffio_read_varlen(bc);

ffio_read_varlen() could move the position beyond maxpos yet return
0 so the loop with teh checks inside is skiped

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


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