[FFmpeg-devel] [PATCH] RTSP-MS 14/15: ASF packet parsing

[Michael Niedermayer]

On Fri, Jul 24, 2009 at 04:39:30PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Jul 20, 2009 at 6:16 PM, Ronald S. Bultje<rsbultje at gmail.com> wrote:
> > On Mon, Jul 20, 2009 at 6:14 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> >> the updated p can have any value the attacker chooses if he can make
> >> len have any value and i think he can but maybe i miss something ...
> >
> > Ah, integer overflows, of course. Will fix.
> 
> See attached, I added a if (end < p) return; at the top, that ensures
> that any access to p is within range and doesn't overflow.
> 
> Ronald

>  Changelog             |    1 
>  libavformat/asfdec.c  |    8 ++
>  libavformat/rtp_asf.c |  189 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  libavformat/rtsp.h    |    4 +
>  4 files changed, 201 insertions(+), 1 deletion(-)
> 45cebe6dfb7e7e5a2a132284ddf6a71f94dad6c0  rtsp-ms-parse-asf-payload.patch
> Index: ffmpeg-svn/libavformat/rtp_asf.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtp_asf.c	2009-07-24 16:32:45.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtp_asf.c	2009-07-24 16:35:47.000000000 -0400
> @@ -27,11 +27,66 @@
>  
>  #include <libavutil/base64.h>
>  #include <libavutil/avstring.h>
> +#include <libavutil/intreadwrite.h>
>  #include "rtp.h"
>  #include "rtp_asf.h"
>  #include "rtsp.h"
>  #include "asf.h"
>  
> +/**
> + * From MSDN 2.2.1.4, we learn that ASF data packets over RTP should not
> + * contain any padding. Unfortunately, the header min/max_pktsize are not
> + * updated (thus making min_pktsize invalid). Here, we "fix" these faulty
> + * min_pktsize values in the ASF file header.
> + */
> +static void
> +rtp_asf_fix_header(uint8_t *buf, int len)
> +{
> +    uint8_t *p = buf, *end = buf + len;
> +
> +    if (end < p || len < sizeof(ff_asf_guid) * 2 + 22 ||
> +        memcmp(p, ff_asf_header, sizeof(ff_asf_guid))) {
> +        return;
> +    }

when can end be < p ?


> +    p += sizeof(ff_asf_guid) + 14;
> +    do {
> +        uint64_t len = AV_RL64(p + sizeof(ff_asf_guid));
> +        if (memcmp(p, ff_asf_file_header, sizeof(ff_asf_guid))) {
> +            p += len;
> +            continue;
> +        }
> +        /* skip most of the file header, to min_pktsize */
> +        p += 6 * 8 + 3 * 4 + sizeof(ff_asf_guid) * 2;
> +        if (p + 8 <= end && AV_RL32(p) == AV_RL32(p + 4)) {
> +            /* and set that to zero */
> +            AV_WL32(p, 0);
> +        }
> +        break;
> +    } while (end - p >= sizeof(ff_asf_guid) + 8);
> +}

this is still buggy, also it can easily end in an infinite loop


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090725/1add2fa4/attachment.pgp>