[FFmpeg-devel] [libav-devel] [PATCH] AMV: Fix possibly exploitable crash.
Aℓex Converse
aconverse at google.com
Fri Apr 29 18:29:04 CEST 2011
On Thu, Apr 28, 2011 at 12:09 PM, Reinhard Tartler <siretart at tauware.de> wrote:
> On Thu, Apr 28, 2011 at 02:36:45PM -0400, Justin Ruggles wrote:
>> On 04/28/2011 01:20 PM, Aℓex Converse wrote:
>>
>> > On Thu, Apr 28, 2011 at 4:32 AM, Reinhard Tartler <siretart at tauware.de> wrote:
>> >> On Wed, Apr 27, 2011 at 03:37:27PM -0400, Justin Ruggles wrote:
>> >>> On 04/27/2011 03:25 PM, Reinhard Tartler wrote:
>> >>>
>> >>>> From: Michael Niedermayer <michaelni at gmx.at>
>> >>>>
>> >>>> Reported-at: Thu, 21 Apr 2011 14:38:25 +0000
>> >>>> Reported-by: Dominic Chell <Dominic.Chell at ngssecure.com>
>> >>>> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >>>> (cherry picked from commit 89f903b3d5ec38c9c5d90fba7e626fa0eda61a32)
>> >>>> (cherry picked from commit 9b919571e506fbb72b81a35ca1e7c1bd6efc4209)
>> >>>> ---
>> >>>> libavcodec/sp5xdec.c | 3 +--
>> >>>> 1 files changed, 1 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> diff --git a/libavcodec/sp5xdec.c b/libavcodec/sp5xdec.c
>> >>>> index e2c371a..3d01020 100644
>> >>>> --- a/libavcodec/sp5xdec.c
>> >>>> +++ b/libavcodec/sp5xdec.c
>> >>>> @@ -86,7 +86,6 @@ static int sp5x_decode_frame(AVCodecContext *avctx,
>> >>>> recoded[j++] = 0xFF;
>> >>>> recoded[j++] = 0xD9;
>> >>>>
>> >>>> - avctx->flags &= ~CODEC_FLAG_EMU_EDGE;
>> >>>> av_init_packet(&avpkt_recoded);
>> >>>> avpkt_recoded.data = recoded;
>> >>>> avpkt_recoded.size = j;
>> >>>> @@ -121,6 +120,6 @@ AVCodec ff_amv_decoder = {
>> >>>> NULL,
>> >>>> ff_mjpeg_decode_end,
>> >>>> sp5x_decode_frame,
>> >>>> - CODEC_CAP_DR1,
>> >>>> + 0,
>> >>>> .long_name = NULL_IF_CONFIG_SMALL("AMV Video"),
>> >>>> };
>> >>>
>> >>>
>> >>> The log message explains nothing. What was the issue? How is it
>> >>> related to CODEC_CAP_DR1 and CODEC_FLAG_EMU_EDGE? Why change
>> >>> ff_amv_decoder and not ff_sp5x_decoder?
>> >>
>> >> No idea, and I'm not able to fill in the missing information. What shall
>> >> we do about this patch now? It seems that it has been picked up by Bugtraq:
>> >> http://seclists.org/bugtraq/2011/Apr/257
>> >>
>> >> And I have a request from the Debian security team to include this
>> >> patch: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=624339
>> >>
>> >
>> >> NGS Secure is going to withhold details of this flaw for three months.
>> >> This three month window will allow users the time needed to apply the
>> >> patch before the details are released to the general public. This
>> >> reflects the NGS Secure approach to responsible disclosure.
>> >
>> > I would complain to NGS about their "responsible disclosure" policy.
>
> I've already asked them nicely for additional information a few hours
> ago. Let's hope they react.
>
>> The EMU_EDGE part looks correct. We should go ahead and commit that. I
>> can't find any reason why CODEC_CAP_DR1 is being turned off for AMV
>> though. I suggest we either ask Michael why this was done or for more
>> information, or we go ahead and comment-out that line (and backport to
>> the 0.6 branch) until NGS Secure releases details of the flaw.
>
> 20:52 <siretart> michaelni: could you please elaborate a bit what the
> dropping of CODEC_CAP_DR1 from the AMV codec actually
> fixes?
> 20:02 <michaelni> siretart, no. A patch that fixes the security bug is
> available, details will not be released before people
> had a chance to apply the patch, because these details
> would possibly help people exlpoit it. That is normal
> & common practice and dominiks bugtraq post also
> explains when details will be released.
>
I've said this before and I'll say it again, what an asshole!
More information about the ffmpeg-devel
mailing list