Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Michael Niedermayer]

On Mon, Nov 20, 2017 at 01:31:06PM -0800, Dale Curtis wrote:
> On Mon, Nov 20, 2017 at 12:34 PM, Michael Niedermayer <
> mich...@niedermayer.cc> wrote:
> 
> > On Mon, Nov 20, 2017 at 12:01:11PM -0800, Dale Curtis wrote:
> > > Derp, that's the wrong comment for this; I meant that for another thread.
> > > Returning an error for optional user data seems a bit harsh, but if
> > that's
> > > what you want, I'm happy to change it. Please let me know if this is the
> > > route you want to take.
> >
> > it could print a warning and continue
> > but there should be something, the build/version is used to do bug
> > workarounds. If that is done incorrectly it can lead to decoding issues
> >
> 
> Printing a warning sounds good to me. Done.
> 
> - dale

>  mpeg4videodec.c |   11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 2c25da550dca9cce06940bdec9b14925f7476fcb  fix_mpeg4_shift_v2.patch
> From 2f0ae7719d20addeac4a8297a64eb57f8df9cdfd Mon Sep 17 00:00:00 2001
> From: Dale Curtis 
> Date: Fri, 17 Nov 2017 16:05:30 -0800
> Subject: [PATCH] [mpeg4video] Fix undefined shift on assumed 8-bit input.
> 
> decode_user_data() attempts to create an integer |build|
> value with 8 bits of spacing for 3 components. However
> each component is an int32_t, so shifting each component
> is undefined for values outside of the 8 bit range.
> 
> This patch simply clamps input to 8-bits per component
> and prints out a warning that the values were clamped.
> 
> Signed-off-by: Dale Curtis 
> ---
>  libavcodec/mpeg4videodec.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

applied

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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

Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Dale Curtis]

On Mon, Nov 20, 2017 at 12:34 PM, Michael Niedermayer <
mich...@niedermayer.cc> wrote:

> On Mon, Nov 20, 2017 at 12:01:11PM -0800, Dale Curtis wrote:
> > Derp, that's the wrong comment for this; I meant that for another thread.
> > Returning an error for optional user data seems a bit harsh, but if
> that's
> > what you want, I'm happy to change it. Please let me know if this is the
> > route you want to take.
>
> it could print a warning and continue
> but there should be something, the build/version is used to do bug
> workarounds. If that is done incorrectly it can lead to decoding issues
>

Printing a warning sounds good to me. Done.

- dale
From 2f0ae7719d20addeac4a8297a64eb57f8df9cdfd Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Fri, 17 Nov 2017 16:05:30 -0800
Subject: [PATCH] [mpeg4video] Fix undefined shift on assumed 8-bit input.

decode_user_data() attempts to create an integer |build|
value with 8 bits of spacing for 3 components. However
each component is an int32_t, so shifting each component
is undefined for values outside of the 8 bit range.

This patch simply clamps input to 8-bits per component
and prints out a warning that the values were clamped.

Signed-off-by: Dale Curtis 
---
 libavcodec/mpeg4videodec.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 76247c3b8c..11d4e08986 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -2153,8 +2153,15 @@ static int decode_user_data(Mpeg4DecContext *ctx, GetBitContext *gb)
 e = sscanf(buf, "FFmpeg v%d.%d.%d / libavcodec build: %d", , , , );
 if (e != 4) {
 e = sscanf(buf, "Lavc%d.%d.%d", , , ) + 1;
-if (e > 1)
-build = (ver << 16) + (ver2 << 8) + ver3;
+if (e > 1) {
+if (ver > 0xFF || ver2 > 0xFF || ver3 > 0xFF) {
+av_log(s->avctx, AV_LOG_WARNING,
+ "Unknown Lavc version string encountered, %d.%d.%d; "
+ "clamping sub-version values to 8-bits.\n",
+ ver, ver2, ver3);
+}
+build = ((ver & 0xFF) << 16) + ((ver2 & 0xFF) << 8) + (ver3 & 0xFF);
+}
 }
 if (e != 4) {
 if (strcmp(buf, "ffmpeg") == 0)
-- 
2.15.0.448.gf294e3d99a-goog

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

Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Michael Niedermayer]

On Mon, Nov 20, 2017 at 12:01:11PM -0800, Dale Curtis wrote:
> Derp, that's the wrong comment for this; I meant that for another thread.
> Returning an error for optional user data seems a bit harsh, but if that's
> what you want, I'm happy to change it. Please let me know if this is the
> route you want to take.

it could print a warning and continue
but there should be something, the build/version is used to do bug
workarounds. If that is done incorrectly it can lead to decoding issues

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

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


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

Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Dale Curtis]

Derp, that's the wrong comment for this; I meant that for another thread.
Returning an error for optional user data seems a bit harsh, but if that's
what you want, I'm happy to change it. Please let me know if this is the
route you want to take.

- dale

On Mon, Nov 20, 2017 at 11:55 AM, Dale Curtis 
wrote:

> On Fri, Nov 17, 2017 at 6:22 PM, Michael Niedermayer <
> mich...@niedermayer.cc> wrote:
>>
>> Not sure what is best but
>> throwing part of the version silently away is not correct
>> most likely erroring out and asking for a sample video to be uploaded
>> would make sense if such a file is encountered
>
>
> I'm using the same workaround used throughout the rest of the file; so
> please clarify if you want me to change this. Happy to change to an error
> w/ patches_welcome if you prefer.
>
> - dale
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Dale Curtis]

On Fri, Nov 17, 2017 at 6:22 PM, Michael Niedermayer  wrote:
>
> Not sure what is best but
> throwing part of the version silently away is not correct
> most likely erroring out and asking for a sample video to be uploaded
> would make sense if such a file is encountered


I'm using the same workaround used throughout the rest of the file; so
please clarify if you want me to change this. Happy to change to an error
w/ patches_welcome if you prefer.

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

Re: [FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Michael Niedermayer]

On Fri, Nov 17, 2017 at 04:07:42PM -0800, Dale Curtis wrote:
> decode_user_data() attempts to create an integer |build|
> value with 8 bits of spacing for 3 components. However
> each component is an int32_t, so shifting each component
> is undefined for values outside of the 8 bit range.
> 
> This patch simply clamps input to 8-bits per component.
> 
> Signed-off-by: Dale Curtis 

>  mpeg4videodec.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 5a181e6ad8a04ea0d3d6c7d08be79243995dc292  fix_mpeg4_shift_v1.patch
> From 0373fed23fb495161267607230e99c8ed36e444a Mon Sep 17 00:00:00 2001
> From: Dale Curtis 
> Date: Fri, 17 Nov 2017 16:05:30 -0800
> Subject: [PATCH] [mpeg4video] Fix undefined shift on assumed 8-bit input.
> 
> decode_user_data() attempts to create an integer |build|
> value with 8 bits of spacing for 3 components. However
> each component is an int32_t, so shifting each component
> is undefined for values outside of the 8 bit range.
> 
> This patch simply clamps input to 8-bits per component.
> 
> Signed-off-by: Dale Curtis 
> ---
>  libavcodec/mpeg4videodec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> index 76247c3b8c..93fa1d9973 100644
> --- a/libavcodec/mpeg4videodec.c
> +++ b/libavcodec/mpeg4videodec.c
> @@ -2154,7 +2154,7 @@ static int decode_user_data(Mpeg4DecContext *ctx, 
> GetBitContext *gb)
>  if (e != 4) {
>  e = sscanf(buf, "Lavc%d.%d.%d", , , ) + 1;
>  if (e > 1)
> -build = (ver << 16) + (ver2 << 8) + ver3;
> +build = ((ver & 0xFF) << 16) + ((ver2 & 0xFF) << 8) + (ver3 & 
> 0xFF);

Not sure what is best but
throwing part of the version silently away is not correct
most likely erroring out and asking for a sample video to be uploaded
would make sense if such a file is encountered


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


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

[FFmpeg-devel] [mpeg4video] Fix undefined shift on assumed 8-bit input.

[Dale Curtis]

decode_user_data() attempts to create an integer |build|
value with 8 bits of spacing for 3 components. However
each component is an int32_t, so shifting each component
is undefined for values outside of the 8 bit range.

This patch simply clamps input to 8-bits per component.

Signed-off-by: Dale Curtis 
From 0373fed23fb495161267607230e99c8ed36e444a Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Fri, 17 Nov 2017 16:05:30 -0800
Subject: [PATCH] [mpeg4video] Fix undefined shift on assumed 8-bit input.

decode_user_data() attempts to create an integer |build|
value with 8 bits of spacing for 3 components. However
each component is an int32_t, so shifting each component
is undefined for values outside of the 8 bit range.

This patch simply clamps input to 8-bits per component.

Signed-off-by: Dale Curtis 
---
 libavcodec/mpeg4videodec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 76247c3b8c..93fa1d9973 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -2154,7 +2154,7 @@ static int decode_user_data(Mpeg4DecContext *ctx, GetBitContext *gb)
 if (e != 4) {
 e = sscanf(buf, "Lavc%d.%d.%d", , , ) + 1;
 if (e > 1)
-build = (ver << 16) + (ver2 << 8) + ver3;
+build = ((ver & 0xFF) << 16) + ((ver2 & 0xFF) << 8) + (ver3 & 0xFF);
 }
 if (e != 4) {
 if (strcmp(buf, "ffmpeg") == 0)
-- 
2.15.0.448.gf294e3d99a-goog

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