New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 359454 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Integer overflow allocating shared memory in AudioInputRendererHost::OnCreateStream

Reported by aaron.st...@gmail.com, Apr 3 2014

Issue description

VULNERABILITY DETAILS
I believe a potential buffer write overflow may occur in the browser process due to an integer overflow condition when allocating shared memory in AudioInputRendererHost::OnCreateStream.

The shared memory in 'entry->shared_memory' is allocated with size 'segment_size * entry->shared_memory_segment_count' bytes.  The variables used in this multiplication are both 32 bit integers that are individually calculated from values provided by the renderer in an AudioInputHostMsg_CreateStream_Config message.  On a multiplication integer overflow, the size of the shared memory allocation may be less than expected by AudioInputSyncWriter::Write, possibly even less than the segment size.  As a result, when AudioInputSyncWriter::Write attempts to write a segment to the shared memory buffer a write overflow may occur.

As a demonstration, I have written a patch simulating a compromised renderer process sending cooked values in an AudioInputHostMsg_CreateStream_Config message.  The result is a 'segment_size' of 131074, a 'entry->shared_memory_segment_count' of 32768, and a shared memory allocation of 65536.  This results in a write of 131058 bytes in AudioInputSyncWriter::Write at an offset from the base address of the 65536 byte shared memory region, which is a write overflow.  The source patch and asan output from a test run are attached.

The bytes written to the overflow memory region will generally come from audio samples.  I believe an attacker might exercise some control over the content of these bytes by manipulating audio recording parameters and the offset of the memory location for the overflow so that any predictable values in the audio samples might be written to a desired location.  A somewhat more remote possibility is that an attacker might attempt to exercise some measure of control over the content of the audio samples by playing specific tones from the computer speaker while the recording is taking place.  Note that I have not experimentally checked the feasibility of either of these methods.

VERSION
Chrome Version:
git source hash 0760c783b421753c2bbeba410d9051c37f728f61
with the attached patch applied for descriptive logging and to simulate a compromised renderer process
64 bit asan release build
Operating System:
OS X 10.9.1 (13B42)
Note that I believe this issue may be present on other operating systems, and in 32 bit compilations as well.

REPRODUCTION CASE
The attached patch was applied to git source hash 0760c783b421753c2bbeba410d9051c37f728f61 and compiled into a 64 bit asan release binary.
The provided audio.html file was served from localhost port 8000 using python -m SimpleHTTPServer.
Chrome was started as follows:
out/Release/Chromium.app/Contents/MacOS/Chromium http://localhost:8000/audio.html &> out.log
After chrome was started, "Allow" was clicked to permit use of the microphone by the audio.html page.
After this, the asan chrome build terminated with the heap buffer overflow reported in the attached asan log.
 
asan.log
9.1 KB View Download
audio.html
127 bytes View Download
patch.diff
3.1 KB Download
Labels: Cr-Internals-Media-Audio
Owner: wjia@chromium.org
Status: Assigned
@wjia - This calculation should be using the CheckedNumeric template from base/numerics.
I also wanted to add that, while in my demonstration the bytes written to the overflow region are audio samples, there is additionally some metadata written to the media::AudioInputBuffer, via the 'params' attribute, in AudioInputSyncWriter::Write.  The contents of this metadata might potentially be easier for an attacker to control than the contents of the audio samples.  If it makes a significant difference I could potentially attempt to write a demo where this metadata is written to an overflow region.
Cc: henrika@chromium.org
Labels: Security_Impact-Beta Security_Impact-Stable Security_Severity-High
It looks like this was introduced in r161328, so it should impact beta and stable.
Cc: xians@chromium.org rtoy@chromium.org

Comment 5 by xians@chromium.org, Apr 3 2014

Cc: dalecur...@chromium.org

+ Dale.

We pass important audio parameters from content via IPC msg to the browser to setup things correct, it will be a design problem if it is possible for the hackers to manipulating the audio parameters.

Can we involve anyone who knows IPC security?
Cc: tsepez@chromium.org
Project Member

Comment 7 by ClusterFuzz, Apr 3 2014

Labels: Pri-1 M-34
You can't trust anything coming in via IPC message.  It must be sanitized.

Comment 9 by wjia@chromium.org, Apr 3 2014

https://codereview.chromium.org/214343006/ has been uploaded for review.
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 3 2014

------------------------------------------------------------------
r261549 | wjia@chromium.org | 2014-04-03T22:23:45.484349Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=261549&r2=261548&pathrev=261549

Check shared memory size before allocating it.

This will prevent overflow from multiplication.

BUG= 359454 

Review URL: https://codereview.chromium.org/214343006
-----------------------------------------------------------------
Status: Fixed
Project Member

Comment 12 by ClusterFuzz, Apr 3 2014

Labels: -Restrict-View-SecurityTeam Merge-Triage Restrict-View-SecurityNotify M-35
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Labels: reward-topanel
Aaron, how would you like to credited in the release notes.
You can just use my name: Aaron Staple
Thanks!
Labels: -Merge-Triage Merge-Approved Merge-Requested
Merge-Approved for M34 (via dxie@).

Merge-Requested for M35.
Labels: -Merge-Approved merge-merged-1847 Release-1-M34
merged to r34 in r264647
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 17 2014

------------------------------------------------------------------
r264647 | inferno@chromium.org | 2014-04-17T21:35:48.300246Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=264647&r2=264646&pathrev=264647

Merge 261549 "Check shared memory size before allocating it."

> Check shared memory size before allocating it.
> 
> This will prevent overflow from multiplication.
> 
> BUG= 359454 
> 
> Review URL: https://codereview.chromium.org/214343006

TBR=wjia@chromium.org

Review URL: https://codereview.chromium.org/241503004
-----------------------------------------------------------------
Merge-Requested for M35.

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 18 2014

------------------------------------------------------------------
r264795 | laforge@chromium.org | 2014-04-18T15:39:10.975944Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=264795&r2=264794&pathrev=264795

Revert 264647
> Merge 261549 "Check shared memory size before allocating it."
> 
> > Check shared memory size before allocating it.
> > 
> > This will prevent overflow from multiplication.
> > 
> > BUG= 359454 
> > 
> > Review URL: https://codereview.chromium.org/214343006
> 
> TBR=wjia@chromium.org
> 
> Review URL: https://codereview.chromium.org/241503004

TBR=inferno@chromium.org
Review URL: https://codereview.chromium.org/243263002
-----------------------------------------------------------------
Labels: -M-34 -merge-merged-1847 -Release-1-M34
Removing merge and release labels. CheckedNumeric did not make the cut into M34 but did make M35 (see Issue 364703), hence the revert.

Spoke with jschuh@ and this can wait for M35.

Comment 22 by kareng@google.com, Apr 21 2014

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 23 by bugdroid1@chromium.org, Apr 21 2014

Labels: -Merge-Approved merge-merged-1916
------------------------------------------------------------------
r265054 | wjia@chromium.org | 2014-04-21T20:22:14.412725Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1916/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=265054&r2=265053&pathrev=265054

Merge 261549 "Check shared memory size before allocating it."

> Check shared memory size before allocating it.
> 
> This will prevent overflow from multiplication.
> 
> BUG= 359454 
> 
> Review URL: https://codereview.chromium.org/214343006

TBR=wjia@chromium.org

Review URL: https://codereview.chromium.org/245753002
-----------------------------------------------------------------
I spent a little bit of time investigating the potential exploitability of this issue, via two separate methods.  (Note the following examples use 32 bit non asan builds on osx).

The fist method was to write a null byte sequence of arbitrary length to a memory region mapped to the right of the audio sample shared memory region.  I carefully chose values sent by the renderer in an AudioInputHostMsg_CreateStream message so that the browser would write 0xbeef bytes from audio samples into the memory region to the right of the shared memory region.  And I added an AudioInputHostMsg_SetVolume message to set the volume to zero, causing all audio data to contain only null bytes.  The memory mapping for this run was:

mapped file            11025000-11047000 [  136K] rw-/rwx SM=ALI  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/.org.chromium.Chromium.lWEajg
MALLOC_LARGE           11047000-11075000 [  184K] rw-/rwx SM=PRV  DefaultMallocZone_0x86c4000

The log is:

segment_size: 188143
entry->shared_memory_segment_count: 251111
segment_size * entry->shared_memory_segment_count: 136617
entry->shared_memory address: 11025000
[New Thread 0x190f of process 53500]
[New Thread 0x3a03 of process 53500]
[New Thread 0x3b03 of process 53500]
[New Thread 0x3c03 of process 53500]

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 0x190f of process 53500]
0x02e37ab9 in ?? ()
(gdb) c
Continuing.
writing zeroes from 0x11025010 to 0x11052eef

Since 0x11052eef - 0x11047000 == 0xbeef, 0xbeef null bytes were written to the MALLOC_LARGE region.

The patch used to generate this run is run1.patch

The second method was an attempt to write arbitrary content to the first word following the audio sample shared memory region, using specially formulated metadata for the ‘params’ attribute of an AudioInputBuffer.  My strategy was to use the two most significant bytes of params.volume and the two least significant bytes of params.size to form a word of configurable content.  Each of the attributes volume and size may be set via ipc form the renderer, but each may be restricted to a specific set of values (at least in this exploit scenario).  Combining bytes from the two attributes results in a larger (though still restricted) set of possible values.

In my test I attempted to write the value 0xbff13fed to the word following the shared memory region.  I tried to do this by overflowing the shared memory size calculation to result in a mapped size equal to shared_memory_segment_count_.  As a result the shared_memory_segment_size_ in AudioInputSyncWriter is calculated as 1, and this causes AudioInputSyncWriter::Write to increase the memory offset it writes to by one byte each time it is called.  So if the shared memory is at address 0x11351000, AudioInputSyncWriter::Write will write an AudioInputParameters followed by many bytes of audio data starting at address 0x11351000.  Then on the next call AudioInputSyncWriter::Write will write an AudioInputParameters followed by many bytes of audio data starting at address 0x11351001, etc, up until address 0x11351000 + mapped_size - 1.  On each such write, the seventh through tenth bytes of the AudioInputParameters comprise the word 0xbff13fed.  Here is the memory mapping for my experimental run:

Here was the memory layout of my test run, showing a MALLOC_LARGE region following the shared memory region:
mapped file            11351000-11391000 [  256K] rw-/rwx SM=ALI  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/.org.chromium.Chromium.XRWYf0
MALLOC_LARGE           11391000-113b8000 [  156K] rw-/rwx SM=PRV  DefaultMallocZone_0x86c4000

Here is an abridged log from my run:

segment_size: 49153
entry->shared_memory_segment_count: 262144
segment_size * entry->shared_memory_segment_count: 262144
entry->shared_memory address: 11351000
set 0x11351006 to 0xbff13fed
set 0x11351007 to 0xbff13fed
set 0x11351008 to 0xbff13fed
set 0x11351009 to 0xbff13fed
set 0x1135100a to 0xbff13fed
set 0x1135100b to 0xbff13fed
…
set 0x11390df3 to 0xbff13fed
set 0x11390df4 to 0xbff13fed
set 0x11390df5 to 0xbff13fed
set 0x11390df6 to 0xbff13fed
set 0x11390df7 to 0xbff13fed

Program received signal SIGBUS, Bus error.
0x11341bb4 in ?? ()
(gdb)

Unfortunately this attempt failed with a SIGBUS before writing 0xbff13fed to my target address 0x11391000.  I believe the sigbus occurred because the MALLOC_LARGE region became corrupted with audio sample data.  Other types of target memory regions may be less susceptible than a heap region to crashing due to corrupt values. However, because of the long runtime of an exploit with a long sequence of AudioInputSyncWriter::Write calls (over a day for this example) I am not planning to investigate further.

The patch used to generate this run is run2.patch.
run1.patch
4.3 KB Download
run2.patch
4.1 KB Download
Cc: jsc...@chromium.org infe...@chromium.org
Labels: Release-0-M35
+inferno@, jschuh@ for c#24
Labels: -reward-topanel reward-unpaid reward-3000 CVE-2014-1744
Project Member

Comment 27 by ClusterFuzz, Jul 11 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 29 by ClusterFuzz, Feb 2 2016

Labels: -Security_Impact-Beta
Project Member

Comment 30 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 31 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment