rtpdec_asf: Fix integer underflow that could allow remote code execution

Message ID 1315402920-57937-1-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö Sept. 7, 2011, 1:42 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

Fixes MSVR-11-0088.
Credit:  Jeong Wook Oh of Microsoft and Microsoft Vulnerability Research (MSVR)

Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
---
 libavformat/rtpdec_asf.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Ronald Bultje Sept. 7, 2011, 1:58 p.m. | #1
Hi,

On Wed, Sep 7, 2011 at 6:42 AM, Martin Storsjö <martin@martin.st> wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
>
> Fixes MSVR-11-0088.
> Credit:  Jeong Wook Oh of Microsoft and Microsoft Vulnerability Research (MSVR)
>
> Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
> ---
>  libavformat/rtpdec_asf.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
> index 287025f..55a8a8a 100644
> --- a/libavformat/rtpdec_asf.c
> +++ b/libavformat/rtpdec_asf.c
> @@ -235,6 +235,8 @@ static int asfrtp_parse_packet(AVFormatContext *s, PayloadContext *asf,
>                 int prev_len = out_len;
>                 out_len += cur_len;
>                 asf->buf = av_realloc(asf->buf, out_len);
> +                if (!asf->buf || FFMIN(cur_len, len - off) < 0)
> +                    return -1;

The patch itself is OK, but while you're at it, also please leave the
old variable alone while testing whether allocation succeeded. Don't
forget, if av_realloc() fails, the old memory stays valid and
allocated and we should not forget about it.

Thus:

if (FFMIN(cur_len, len - off) < 0)
    return -1;
void *newmem = av_realloc(asf->buf, out_len);
if (!newmem)
    return -1;

Or something along those lines. Thanks for fixing this one, btw.

Ronald

Patch

diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
index 287025f..55a8a8a 100644
--- a/libavformat/rtpdec_asf.c
+++ b/libavformat/rtpdec_asf.c
@@ -235,6 +235,8 @@  static int asfrtp_parse_packet(AVFormatContext *s, PayloadContext *asf,
                 int prev_len = out_len;
                 out_len += cur_len;
                 asf->buf = av_realloc(asf->buf, out_len);
+                if (!asf->buf || FFMIN(cur_len, len - off) < 0)
+                    return -1;
                 memcpy(asf->buf + prev_len, buf + off,
                        FFMIN(cur_len, len - off));
                 avio_skip(pb, cur_len);