[FFmpeg-devel] [libav-devel] [PATCH] AMV: Fix possibly exploitable crash.

[Reimar Döffinger]

On Fri, Apr 29, 2011 at 09:29:04AM -0700, Aℓex Converse wrote:
> 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!

What is so horribly unreasonable about this that it justifies your
word choice?
Even the maintainers of the Linux kernel stable branches usually
don't mention details about security issues.
Particularly when asking on a public channel, if it was via
signed and encrypted email that might change my opinion a bit.