New issue
Advanced search Search tips

Issue 839960 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 8
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-09
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Use of uninitialized memory caused by AcmReceiver::AcmReceiver()

Reported by googleb...@zippenhop.com, May 4

Issue description

[This is a WebRTC bug, that I am filing here because there doesn't appear to be a security template at https://bugs.chromium.org/p/webrtc/issues/entry]

AcmReceiver::AcmReceiver() (modules\audio_coding\acm2\acm_receiver.cc) causes other code in its class to use uninitialized memory by failing fully to initialize a buffer. Line 41, below, initializes only the first half of |last_audio_buffer_|, since, per line 36, the buffer is actually |AudioFrame::kMaxDataSizeSamples * sizeof (int16_t)| bytes long:

35: AcmReceiver::AcmReceiver(const AudioCodingModule::Config& config)
36:     : last_audio_buffer_(new int16_t[AudioFrame::kMaxDataSizeSamples]),
37:       neteq_(NetEq::Create(config.neteq_config, config.decoder_factory)),
38:       clock_(config.clock),
39:       resampled_last_output_frame_(true) {
40:   assert(clock_);
41:   memset(last_audio_buffer_.get(), 0, AudioFrame::kMaxDataSizeSamples);
42: }

I found this bug in the context of Firefox 59.0.2.

You can see the bug in action by starting Firefox, attaching a debugger to it, and putting a BP on line 36. Now go to https://webrtc.github.io/samples/src/content/peerconnection/audio, click "call", and let Firefox use your microphone. When the BP fires, look at the disassembly and notice that line 36 allocates 0x1e00 bytes of memory. Then step line 41 and see that it initializes only 0xf00 bytes of that memory.

This failure can cause a data leak.

On Firefox, this bug is latent because Firefox's allocator appears always to initializes heap blocks, but other codebases might not do that.

This bug is still present in Master: https://webrtc.googlesource.com/src/+/master/modules/audio_coding/acm2/acm_receiver.cc . The Firefox source for the module is in media\webrtc\trunk\webrtc\modules\audio_coding\acm2\acm_receiver.cc .
 
Components: Blink>WebRTC>Audio
Labels: Security_Severity-Medium M-67 Security_Impact-Stable OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows Pri-2
Owner: tommi@chromium.org
Project Member

Comment 2 by sheriffbot@chromium.org, May 5

Labels: -Pri-2 Pri-1
Project Member

Comment 3 by sheriffbot@chromium.org, May 5

Status: Assigned (was: Unconfirmed)
Owner: hlundin@chromium.org
Henrik - can you take a look?
Status: Started (was: Assigned)
Thanks for reporting. I'm on it.

Cc: kwiberg@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, May 8

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/76c106725a227c6c1221356a303d007d707b0a45

commit 76c106725a227c6c1221356a303d007d707b0a45
Author: Henrik Lundin <henrik.lundin@webrtc.org>
Date: Tue May 08 11:40:04 2018

ACM: Properly initialize last_audio_buffer_ array

Only half of the array was initialized. Now all of it is.

Bug:  chromium:839960 
Change-Id: If8bbe12c4c4c0dc0d529c93b22e49a94ecb09919
Reviewed-on: https://webrtc-review.googlesource.com/74820
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23167}
[modify] https://crrev.com/76c106725a227c6c1221356a303d007d707b0a45/modules/audio_coding/acm2/acm_receiver.cc

Status: Fixed (was: Started)
Labels: Merge-Request-67
Project Member

Comment 10 by sheriffbot@chromium.org, May 8

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Cc: awhalley@chromium.org
+awhalley@ for M67 merge review (CL listed at #7 needs canary baking, landed in trunk 3 hrs back)
Project Member

Comment 12 by bugdroid1@chromium.org, May 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df04b893a4d29cdb1f48473735a33d998d5e1d76

commit df04b893a4d29cdb1f48473735a33d998d5e1d76
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue May 08 16:01:46 2018

Roll src/third_party/webrtc/ 826738b78..5b2b69207 (7 commits)

https://webrtc.googlesource.com/src.git/+log/826738b78c6a..5b2b692079e6

$ git log 826738b78..5b2b69207 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:none,chromium:None,chromium:839960


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I45f3d07c607b57730126b195af1cffa4a8064f87
Reviewed-on: https://chromium-review.googlesource.com/1049954
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#556811}
[modify] https://crrev.com/df04b893a4d29cdb1f48473735a33d998d5e1d76/DEPS

NextAction: 2018-05-09
hlundin@@, pls update the bug with canary result tomorrow. Thank you.
The NextAction date has arrived: 2018-05-09
Project Member

Comment 15 by sheriffbot@chromium.org, May 9

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Any update on canary result?
The fix was included in 68.0.3425.0, which was released in Win and Mac canary channels some 14 hours ago. I guess it is a bit early to tell what the result was.

Also, since this particular issue wasn't found through any of Chrome's or WebRTC's regular monitoring tools, but rather as a report from a Firefox developer, I doubt that we will be able to see any change in our stats based on the fix.

We can come back next week and revisit the the data if you want to.

Thank you  hlundin@.
awhalley@ (Security TPM) will be doing M67 merge review (Ptal comment #17).

govind@ - good for 67
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #19. Pls merge by Monday morning MTV time.
Labels: -Merge-Approved-67 merge-merged-67
Merged to M67 in https://webrtc-review.googlesource.com/c/src/+/76641.
Labels: reward-topanel
Labels: -reward-topanel reward-unpaid reward-500
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
I reported this bug to Mozilla, which uses this library in Firefox, prior to reporting it here. I have not disclosed it elsewhere.
The VRP panel decided to award $500 for this report, thanks! How would you like to be credited in release notes?
Labels: -reward-unpaid reward-inprocess
> The VRP panel decided to award $500 for this report, thanks! How would you like 
> to be credited in release notes?

Thank you. Please credit me as "Ronald E. Crane".
Labels: Release-0-M67
Labels: CVE-2018-6132 CVE_description-missing
Project Member

Comment 30 by sheriffbot@chromium.org, Aug 14

Labels: -Restrict-View-SecurityNotify 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