Re: [FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count
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
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
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
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
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
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
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