Quoting Andreas Cadhalpun (2015-02-26 01:27:34)
> On 26.02.2015 00:24, Michael Niedermayer wrote:
> > On Wed, Feb 25, 2015 at 11:48:33PM +0100, Andreas Cadhalpun wrote:
> >> Hi,
> >>
> >> if avctx->channels is 0 in adx_read_packet, size gets set to 0,
> >> av_get_packet sets pkt->data to NULL and then AV_RB16(pkt->data)
> >> results in a null pointer dereference.
> >>
> >> Attached patch fixes this.
> >>
> >> Best regards,
> >> Andreas
> >
> >>   adxdec.c |    5 +++++
> >>   1 file changed, 5 insertions(+)
> >> 7312e6a3be1771c83eac72784496c6fc4692d954  
> >> 0001-avformat-adxdec-check-avctx-channels-for-invalid-val.patch
> >>  From 2578976a0a9eec03d168f393795119fd274ee81f Mon Sep 17 00:00:00 2001
> >> From: Andreas Cadhalpun <[email protected]>
> >> Date: Wed, 25 Feb 2015 22:55:44 +0100
> >> Subject: [PATCH] avformat/adxdec: check avctx->channels for invalid values
> >>
> >> This avoids a null pointer dereference of pkt->data.
> >>
> >> Signed-off-by: Andreas Cadhalpun <[email protected]>
> >> ---
> >>   libavformat/adxdec.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/libavformat/adxdec.c b/libavformat/adxdec.c
> >> index ddaa201..24a8a1f 100644
> >> --- a/libavformat/adxdec.c
> >> +++ b/libavformat/adxdec.c
> >> @@ -40,6 +40,11 @@ static int adx_read_packet(AVFormatContext *s, AVPacket 
> >> *pkt)
> >>       AVCodecContext *avctx = s->streams[0]->codec;
> >>       int ret, size;
> >>
> >> +    if (avctx->channels <= 0) {
> >> +        av_log(s, AV_LOG_ERROR, "invalid number of channels %d\n", 
> >> avctx->channels);
> >> +        return AVERROR_INVALIDDATA;
> >> +    }
> >
> > the demuxer should extract the channel value in adx_read_header()
> > and check it there. (if it needs the channels, which it does currently)
> >
> > its not good for demuxing to depend on a decoder/parser setting this
> > value between reading the file header and before demuxing the first
> > packet
> 
> You're right about that. Attached is a patch for this.
> 
> However it might still be a good idea to apply above patch, because the 
> decoder/parser could set avctx->channels to 0, even if the demuxer has set it 
> to 
> something positive.

Yes, that's a pervasive problem I'm going to address in a rather huge
set I'm working on. In the meantime, I think it'd be best to just store
the value in the private data to make sure no one messes with it.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to