| Message ID | MN2PR04MB5981843EDD8F31B84CE9AC7ABAF49@MN2PR04MB5981.namprd04.prod.outlook.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [FFmpeg-devel] libavformat/asfdec: Fix regression bug when reading image attachments | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works <softworkz@hotmail.com>: > > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for value_len > UINT16_MAX. > As a consequence, attached images of sizes larger than UINT16_MAX could no longer be read. > > Signed-off-by: softworkz <softworkz@hotmail.com> > --- > libavformat/asfdec_f.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c > index f784e62996..708331637e 100644 > --- a/libavformat/asfdec_f.c > +++ b/libavformat/asfdec_f.c > @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size) > { > AVIOContext *pb = s->pb; > ASFContext *asf = s->priv_data; > - int n, stream_num, name_len_utf16, name_len_utf8, value_len; > + int n, stream_num, name_len_utf16, name_len_utf8; > + unsigned int value_len; There is something wrong with the indentation afaict. And why can't you fix the issue leaving the variable an int? Carl Eugen
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Sunday, 8 August 2021 01:33 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug > when reading image attachments > > Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works > <softworkz@hotmail.com>: > > > > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a > check for value_len > UINT16_MAX. > > As a consequence, attached images of sizes larger than UINT16_MAX could > no longer be read. > > > > Signed-off-by: softworkz <softworkz@hotmail.com> > > --- > > libavformat/asfdec_f.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index > > f784e62996..708331637e 100644 > > --- a/libavformat/asfdec_f.c > > +++ b/libavformat/asfdec_f.c > > @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, > > int64_t size) { > > AVIOContext *pb = s->pb; > > ASFContext *asf = s->priv_data; > > - int n, stream_num, name_len_utf16, name_len_utf8, value_len; > > + int n, stream_num, name_len_utf16, name_len_utf8; > > + unsigned int value_len; > > There is something wrong with the indentation afaict. You're right, wrong IDE setting. Thanks. > And why can't you fix the issue leaving the variable an int? Why use an int variable to take the value of a function that returns uint and afterwards check whether it's negative? Why use int type for size values at all? I didn't want to make a bigger thing out of the fix, though. softworkz
Am So., 8. Aug. 2021 um 01:33 Uhr schrieb Carl Eugen Hoyos <ceffmpeg@gmail.com>: > > Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works <softworkz@hotmail.com>: > > > > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for value_len > UINT16_MAX. > > As a consequence, attached images of sizes larger than UINT16_MAX could no longer be read. > > > > Signed-off-by: softworkz <softworkz@hotmail.com> > > --- > > libavformat/asfdec_f.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c > > index f784e62996..708331637e 100644 > > --- a/libavformat/asfdec_f.c > > +++ b/libavformat/asfdec_f.c > > @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size) > > { > > AVIOContext *pb = s->pb; > > ASFContext *asf = s->priv_data; > > - int n, stream_num, name_len_utf16, name_len_utf8, value_len; > > + int n, stream_num, name_len_utf16, name_len_utf8; > > + unsigned int value_len; > > There is something wrong with the indentation afaict. > And why can't you fix the issue leaving the variable an int? Should have been: Is it possible to fix the issue without changing the variable type? Sorry for the bad wording, Carl Eugen
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index f784e62996..708331637e 100644 --- a/libavformat/asfdec_f.c +++ b/libavformat/asfdec_f.c @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size) { AVIOContext *pb = s->pb; ASFContext *asf = s->priv_data; - int n, stream_num, name_len_utf16, name_len_utf8, value_len; + int n, stream_num, name_len_utf16, name_len_utf8; + unsigned int value_len; int ret, i; n = avio_rl16(pb); @@ -721,7 +722,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size) value_type = avio_rl16(pb); /* value_type */ value_len = avio_rl32(pb); - if (value_len < 0 || value_len > UINT16_MAX) + if (value_len > INT32_MAX) return AVERROR_INVALIDDATA; name_len_utf8 = 2*name_len_utf16 + 1; @@ -743,7 +744,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size) if(stream_num < 128) asf->dar[stream_num].den = aspect_y; } else { - get_tag(s, name, value_type, value_len, 16); + get_tag(s, name, value_type, (int)value_len, 16); } av_freep(&name); }
Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for value_len > UINT16_MAX. As a consequence, attached images of sizes larger than UINT16_MAX could no longer be read. Signed-off-by: softworkz <softworkz@hotmail.com> --- libavformat/asfdec_f.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)