[FFmpeg-devel] [libav-devel] [PATCH 2/2] matroskadec: validate audio channels and bitdepth

[Michael Niedermayer]

On Tue, Jun 16, 2015 at 12:09:37AM +0200, Andreas Cadhalpun wrote:
> On 15.06.2015 21:40, Hendrik Leppkes wrote:
> > On Mon, Jun 15, 2015 at 9:17 PM, Andreas Cadhalpun
> > <andreas.cadhalpun at googlemail.com> wrote:
> >> The values are written with avio_wl16 and if they don't fit into
> >> uint16_t, this triggers an av_assert2 in avio_w8.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavformat/matroskadec.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> >> index 81dd53f..7af03c9 100644
> >> --- a/libavformat/matroskadec.c
> >> +++ b/libavformat/matroskadec.c
> >> @@ -1889,6 +1889,14 @@ static int matroska_parse_tracks(AVFormatContext *s)
> >>                                NULL, NULL, NULL, NULL);
> >>              avio_write(&b, "TTA1", 4);
> >>              avio_wl16(&b, 1);
> >> +            if (track->audio.channels > UINT16_MAX ||
> >> +                track->audio.bitdepth > UINT16_MAX) {
> >> +                av_log(matroska->ctx, AV_LOG_ERROR,
> >> +                       "Too large audio channel number %"PRIu64
> >> +                       " or bitdepth %"PRIu64".\n",
> >> +                       track->audio.channels, track->audio.bitdepth);
> >> +                return AVERROR_INVALIDDATA;
> >> +            }
> >>              avio_wl16(&b, track->audio.channels);
> >>              avio_wl16(&b, track->audio.bitdepth);
> >>              if (track->audio.out_samplerate < 0 || track->audio.out_samplerate > INT_MAX)
> >> --
> > 
> > The commit message could clarify that this is in the TTA extradata
> > re-construction, because I was briefly confused why a demuxer would
> > "write" the sample rate.
> 
> OK.
> 
> > Additionally, I would vote for using continue and just leaving this
> > track broken, instead of erroring out of reading entirely and making
> > reading the entire file impossible.
> 
> I guess you're also fine with an explode mode.
> Updated patch attached.
> 
> Best regards,
> Andreas
> 

>  matroskadec.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> c84b48d9c49779669de96671e178dd1f6da215d9  0002-matroskadec-validate-audio-channels-and-bitdepth.patch
> From 903de886ae73469831f3416d5fc57c2a6ab97708 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Mon, 15 Jun 2015 21:06:51 +0200
> Subject: [PATCH 2/2] matroskadec: validate audio channels and bitdepth
> 
> In the TTA extradata re-construction the values are written with
> avio_wl16 and if they don't fit into uint16_t, this triggers an
> av_assert2 in avio_w8.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavformat/matroskadec.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

LGTM

thanks

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150616/f02b039e/attachment.asc>