New issue
Advanced search Search tips

Issue 713639 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

FFMPEG function w64_read_header is lack of a necessary bound check on uint32_t.

Reported by ruc.i...@gmail.com, Apr 20 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36

Steps to reproduce the problem:
FFmpeg libavformat/wavdec.c
function w64_read_header
When w64 header contains a chunk_size == UINT_MAX, subsequently a memory corruption may appear.

What is the expected behavior?

What went wrong?
Source file:      wavdec.c
chrome! w64_read_header

The function w64_read_header of Wave64 demuxer is lack of a bound check on a variable from w64 header which can result in av_malloc(0). As a result, only one unit is allocated, subsequently followed by a memory corruption of writing to unallocated heap space.

If the non-negative chunk_size = 4GB is read and immediately passed to “av_alloc(chunk_size + 1)”, then av_malloc will get a parameter which is equals to 0.

static int w64_read_header(AVFormatContext *s)
{
    	...
            uint32_t count, chunk_size, i;

            start = avio_tell(pb);
            end = start + FFALIGN(size, INT64_C(8)) - 24;
            count = avio_rl32(pb);

            for (i = 0; i < count; i++) {
                char chunk_key[5], *value;

                if (avio_feof(pb) || (cur = avio_tell(pb)) < 0 || cur > end - 8 /* = tag + size */)
                    break;

                chunk_key[4] = 0;
                avio_read(pb, chunk_key, 4);
                chunk_size = avio_rl32(pb);   //< -------------------if chunk_size == 0xFFFFFFFF

                value = av_mallocz(chunk_size + 1);  //< ---------results in a allocation to value of size == 0
                if (!value)
                    return AVERROR(ENOMEM);

                ret = avio_get_str16le(pb, chunk_size, value, chunk_size);  //< -------reading data from pb to value results in corruption

Recommending patch:
                avio_read(pb, chunk_key, 4);
                chunk_size = avio_rl32(pb);
+          if (chunk_size == UINT_MAX) {
+			return AVERROR_INVALIDDATA;
+		}
                 value = av_mallocz(chunk_size + 1);

Did this work before? N/A 

Chrome version: 53.0.2785.143  Channel: beta
OS Version: Ubuntu 16.04
Flash Version: Shockwave Flash 23.0 r0
 
Components: Internals>Media>FFmpeg
This appears to still exist in the current m60 code: https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavformat/wavdec.c?l=850&rcl=28a5cdde5c32bcf66715343c10f74e85713f7aaf


Comment 2 by mea...@chromium.org, Apr 21 2017

Labels: Security_Impact-Stable Security_Severity-High
Owner: tguilbert@chromium.org
Status: Assigned (was: Unconfirmed)
tguilbert: Can you please take a look or reassign as appropriate? Thanks.
Project Member

Comment 3 by sheriffbot@chromium.org, Apr 22 2017

Labels: M-58
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 22 2017

Labels: -Pri-2 Pri-1

Comment 5 by kenrb@chromium.org, May 1 2017

tguilbert: Can you please assess whether this is valid, and whether this is as straightforward to fix as it looks?
Status: WontFix (was: Assigned)
It seems like a valid fix to a valid problem. However, in chromium, we do not support Wave64 and we are unlikely to do so. This code is behind the an #if defined(CONFIG_W64_DEMUXER), and we do not enable CONFIG_W64_DEMUXER.

It is therefore not a chromium security issue.

I will follow up with original submitter and make sure that this is upstreamed in FFmpeg.
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 8 2017

Labels: -Restrict-View-SecurityTeam allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment