Issue metadata
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
,
Apr 21 2017
tguilbert: Can you please take a look or reassign as appropriate? Thanks.
,
Apr 22 2017
,
Apr 22 2017
,
May 1 2017
tguilbert: Can you please assess whether this is valid, and whether this is as straightforward to fix as it looks?
,
May 1 2017
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.
,
Aug 8 2017
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 |
|||||||||||||||||||||
Comment 1 by elawrence@chromium.org
, Apr 20 2017