Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 9 Dec 2018 18:11:00 -0500
From: Ren Kimura <rkx1209dev@...il.com>
To: oss-security@...ts.openwall.com, matthew.fernandez@...il.com
Subject: Re: mpg321: Out-of-bounds Write

> Did you report this one upstream? In trying to understand this, it looks to me like the problem isn’t that mpg321 fails
> to check the bitrate is positive, but rather that there’s an unchecked malloc elsewhere.
>
> The point where the OOB write occurs (mad.c:285) looks like the following:
>
>    282     /* update cached table of frames & times */
>    283     if (current_frame <= playbuf->num_frames) /* we only allocate enough for our estimate. */
>    284     {
>    285         playbuf->frames[current_frame] = playbuf->frames[current_frame-1] + (header->bitrate / 8 / 1000)
>    286             * mad_timer_count(header->duration, MAD_UNITS_MILLISECONDS);
>    287         playbuf->times[current_frame] = current_time;
>
> At this point, header->bitrate is 0 and playbuf->num_frames is the correct limit to check against for this buffer. The
> problem seems to stem from the point at which playbuf->frames was allocated (mpg321.c:990):

>    985             if ((options.maxframes != -1) && (options.maxframes <= playbuf.num_frames))
>    986             {
>    987                 playbuf.max_frames = options.maxframes;
>    988             }
>    989
>    990             playbuf.frames = malloc((playbuf.num_frames + 1) * sizeof(void*));
>    991             playbuf.times = malloc((playbuf.num_frames + 1) * sizeof(mad_timer_t));
>    992 #ifdef __uClinux__
>    993       if((playbuf.buf = mmap(0, playbuf.length, PROT_READ, MAP_PRIVATE, fd, 0)) == MAP_FAILED)
>    994 #else
>    995       if((playbuf.buf = mmap(0, playbuf.length, PROT_READ, MAP_SHARED, fd, 0)) == MAP_FAILED)
>    996 #endif
>
> At this point, playbuf.num_frames is whatever the your platform happens to yield when ∞ is cast to a long (undefined
> behavior in C). AFAICT there is no check that malloc succeeded before the code later writes to the frames array (the
> same applies to playbuf.times). Poking around a bit more, this (unchecked malloc) seems common in the code.

checking malloc status is not enough, because playbuf.num_frames can
be very large value, in my environment Ubuntu 18.04, gcc 7.03,
it becomes 0x8000000000000000.
990             playbuf.frames = malloc((playbuf.num_frames + 1) *
sizeof(void*));
So at this point it try to calculate (0x8000000000000000 + 1) * 8 =
0x8 (INTEGER OVERFLOW).
As a result malloc succeed but it only allocate 0x8 byte buffer, lead
OOB write at following points.

283     if (current_frame <= playbuf->num_frames) /* we only allocate
enough for our estimate. */
285         playbuf->frames[current_frame] =
playbuf->frames[current_frame-1] + (header->bitrate / 8 / 1000)
286             * mad_timer_count(header->duration, MAD_UNITS_MILLISECONDS);
287         playbuf->times[current_frame] = current_time;

The value of playbuf.num_frames may depend on platform because it's
calculated from INF value. (undefined behavior)
I only tried to Ubuntu package of mpg321 (may be compiled by gcc?). At
least on ubuntu, OOB write always happen due to above reason.

Ren Kimura

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.