Closed
Bug 1368490
(CVE-2017-7758)
Opened 7 years ago
Closed 7 years ago
Write past end of allocation in InterleaveTrackData
Categories
(Core :: Audio/Video: Playback, defect)
Tracking
()
People
(Reporter: ntrippar, Assigned: bryce)
References
Details
(Keywords: csectype-bounds, reporter-external, sec-high, Whiteboard: [post-critsmash-triage][adv-main54+][adv-esr52.2+])
Attachments
(3 files, 1 obsolete file)
625 bytes,
text/html
|
Details | |
1.83 KB,
patch
|
jesup
:
review+
abillings
:
approval-mozilla-beta+
abillings
:
approval-mozilla-esr52+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
2.01 KB,
patch
|
jesup
:
review+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36
Steps to reproduce:
open the file and it will crash. now you added the vector accessor checks. in tor browser in my last test was maybe exploitable because didnt have that check
Actual results:
=================================================================
==23790==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000051b5b6 bp 0x7fa3d2c89e50 sp 0x7fa3d2c89ce0 T25)
==23790==The signal is caused by a WRITE memory access.
==23790==Hint: address points to the zero page.
#0 0x51b5b5 in MOZ_CrashPrintf /home/worker/workspace/build/src/mfbt/Assertions.cpp:63:3
#1 0x7fa4370e5ebf in InvalidArrayIndex_CRASH(unsigned long, unsigned long) /home/worker/workspace/build/src/xpcom/ds/nsTArray.cpp:26:3
#2 0x7fa43c182296 in ElementAt /home/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1043:7
#3 0x7fa43c182296 in operator[] /home/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1081
#4 0x7fa43c182296 in mozilla::AudioTrackEncoder::InterleaveTrackData(mozilla::AudioChunk&, int, unsigned int, float*) /home/worker/workspace/build/src/dom/media/encoder/TrackEncoder.cpp:151
#5 0x7fa43c17dfd7 in mozilla::OpusTrackEncoder::GetEncodedTrack(mozilla::EncodedFrameContainer&) /home/worker/workspace/build/src/dom/media/encoder/OpusTrackEncoder.cpp:349:9
#6 0x7fa43c1794a6 in mozilla::MediaEncoder::WriteEncodedDataToMuxer(mozilla::TrackEncoder*) /home/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:344:32
#7 0x7fa43c1779f5 in mozilla::MediaEncoder::GetEncodedData(nsTArray<nsTArray<unsigned char> >*, nsAString&) /home/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:290:12
#8 0x7fa43c0aa574 in mozilla::dom::MediaRecorder::Session::Extract(bool) /home/worker/workspace/build/src/dom/media/MediaRecorder.cpp:620:15
#9 0x7fa43c0aa0aa in mozilla::dom::MediaRecorder::Session::ExtractRunnable::Run() /home/worker/workspace/build/src/dom/media/MediaRecorder.cpp:278:19
#10 0x7fa4371bd02e in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1321:14
#11 0x7fa4371c8db8 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:472:10
#12 0x7fa437f6e800 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:338:20
#13 0x7fa437ecae00 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
#14 0x7fa437ecae00 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
#15 0x7fa437ecae00 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
#16 0x7fa4371b5502 in nsThread::ThreadFunc(void*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:501:11
#17 0x7fa4517d8453 in _pt_root /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:216:5
#18 0x7fa45519f6d9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76d9)
#19 0x7fa45422517e in clone /build/glibc-cxyGtm/glibc-2.24/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:105
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/worker/workspace/build/src/mfbt/Assertions.cpp:63:3 in MOZ_CrashPrintf
Thread T25 (Media_Encoder) created by T0 (Web Content) here:
#0 0x4a3d56 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:245:3
#1 0x7fa4517d51f9 in _PR_CreateThread /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:457:14
#2 0x7fa4517d4e0e in PR_CreateThread /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:548:12
#3 0x7fa4371b7737 in nsThread::Init(nsACString const&) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:682:8
#4 0x7fa4371c1fff in nsThreadManager::NewNamedThread(nsACString const&, unsigned int, nsIThread**) /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:273:22
#5 0x7fa4371c4213 in NS_NewNamedThread(nsACString const&, nsIThread**, nsIRunnable*, unsigned int) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:113:45
#6 0x7fa43c0a5a2a in NS_NewNamedThread<14> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:72:10
#7 0x7fa43c0a5a2a in mozilla::dom::MediaRecorder::Session::InitEncoder(unsigned char, int) /home/worker/workspace/build/src/dom/media/MediaRecorder.cpp:771
#8 0x7fa43c0a6b3f in mozilla::dom::MediaRecorder::Session::TracksAvailableCallback::NotifyTracksAvailable(mozilla::DOMMediaStream*) /home/worker/workspace/build/src/dom/media/MediaRecorder.cpp:355:17
#9 0x7fa43be42753 in mozilla::DOMMediaStream::CheckTracksAvailable() /home/worker/workspace/build/src/dom/media/DOMMediaStream.cpp:1287:19
#10 0x7fa43be5f112 in applyImpl<mozilla::DOMMediaStream, void (mozilla::DOMMediaStream::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1084:12
#11 0x7fa43be5f112 in apply<mozilla::DOMMediaStream, void (mozilla::DOMMediaStream::*)()> /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1090
#12 0x7fa43be5f112 in mozilla::detail::RunnableMethodImpl<mozilla::DOMMediaStream*, void (mozilla::DOMMediaStream::*)(), true, (mozilla::RunnableKind)0>::Run() /home/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1133
#13 0x7fa4371bd02e in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1321:14
#14 0x7fa4371c8db8 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:472:10
#15 0x7fa437f6d461 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
#16 0x7fa437ecae00 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
#17 0x7fa437ecae00 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
#18 0x7fa437ecae00 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
#19 0x7fa43d37b80f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:27
#20 0x7fa440c0eea7 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#21 0x7fa437ecae00 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
#22 0x7fa437ecae00 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
#23 0x7fa437ecae00 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
#24 0x7fa440c0e909 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:709:34
#25 0x4eb7a3 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30
#26 0x4eb7a3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:285
#27 0x7fa45413d3f0 in __libc_start_main /build/glibc-cxyGtm/glibc-2.24/csu/../csu/libc-start.c:291
==23790==ABORTING
Comment 1•7 years ago
|
||
Maire or Chris, can you forward to the relevant folks? Thanks!
Group: firefox-core-security → core-security
Component: Untriaged → Audio/Video: Playback
Flags: needinfo?(mreavy)
Flags: needinfo?(cpearce)
Product: Firefox → Core
Comment 2•7 years ago
|
||
Bryce, can you look at this?
Flags: needinfo?(mreavy)
Flags: needinfo?(cpearce)
Flags: needinfo?(bvandyk)
Assignee | ||
Comment 4•7 years ago
|
||
The issue here is that the OpusEncoder sets mChannels at Init, but the number of channels on the stream to the recorder can change in that encoder's lifetime without the encoder being updated. The AudioTrackEncoder does not anticipate this change and if newNumChannels < oldNumChannels then we end up reading out of bounds (specifically off the end of aChunk.mChannelData). This is easily reproducible as shown by the proof of concept. The out of bounds data is read into an array that expects audio channel data. It looks to me like this data will be interleaved and then fed through the opus encoder before being exposed by the media recorder API.
Would appreciate checking of my working. At this stage I think the upshot is exposure of memory (whatever follows aChunk.mChannelData), this data is fed through the opus encoder before being exposed.
Have a WIP fix, to follow shortly for feedback.
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
Comment on attachment 8872553 [details] [diff] [review]
Make sure number of channels is correctly set for opus encoder
I don't think it makes sense to change the number of channels mid flight for opus (webm doesn't support this, and the opus encoder/decoder don't appear to). As such I suggest we keep the same encoder, and up or down sample to maintain the same number of channels. I've run a couple of test cases over chrome, who appear to do this.
We could also stop recording, but I prefer the above as more user friendly, barring any concerns.
Attachment #8872553 -
Flags: review?(pehrson)
Comment 7•7 years ago
|
||
Comment on attachment 8872553 [details] [diff] [review]
Make sure number of channels is correctly set for opus encoder
Review of attachment 8872553 [details] [diff] [review]:
-----------------------------------------------------------------
The code looks good as far as avoiding a crash goes.
What happens when the number of channels increases? Do we know which channels to pick and encode?
What happens when the number of channels decreases? Do we encode silence or garbage on the ones that didn't make the cut?
Would it make more sense to abort and raise an error event instead?
Also write a crash test before landing please. The attachment would be easy to convert.
Attachment #8872553 -
Flags: review?(pehrson)
Updated•7 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 8•7 years ago
|
||
Will do on the crash test.
Does the existing upmix and downmix when interleaving code handle those cases? Would you prefer the error approach? I'm not sure which is more intuitive/useful to the user. If they're expecting a channel # change in the output file then it could be misleading to not error. However, an error would break web app compat between us and chrome.
Are these questions you'd like resolved as part of this bug, or food for thought going forward?
Updated•7 years ago
|
Flags: sec-bounty?
Comment 9•7 years ago
|
||
(In reply to Bryce Van Dyk (:SingingTree) from comment #8)
> Does the existing upmix and downmix when interleaving code handle those
> cases? Would you prefer the error approach? I'm not sure which is more
> intuitive/useful to the user. If they're expecting a channel # change in the
> output file then it could be misleading to not error. However, an error
> would break web app compat between us and chrome.
Is there even existing code to upmix and downmix? I see that we support mono and stereo only, but nothing about dynamic changes. Is that even supported by opus? If we want to support this we IMHO need a set of gtests to prove its functionality.
My first thought is to follow the spec. If the spec doesn't mention what to do, and what to do isn't obvious, we should raise an error to show that the feature is unsupported and no further thought has been given on the behavior. Like we did for video resolution changes until we added support and tests.
IMO compat is secondary here since it's not in the spec, and it's not a regression.
Comment 10•7 years ago
|
||
Thanks, Chris and Bryce, for handling this.
Comment 11•7 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #9)
> (In reply to Bryce Van Dyk (:SingingTree) from comment #8)
> > Does the existing upmix and downmix when interleaving code handle those
> > cases? Would you prefer the error approach? I'm not sure which is more
> > intuitive/useful to the user. If they're expecting a channel # change in the
> > output file then it could be misleading to not error. However, an error
> > would break web app compat between us and chrome.
>
> Is there even existing code to upmix and downmix? I see that we support mono
> and stereo only, but nothing about dynamic changes. Is that even supported
> by opus? If we want to support this we IMHO need a set of gtests to prove
> its functionality.
For audio playback and other reasons, we have dynamic up/down-mixers, including this one.
It handles channels on a chunk-by-chunk basis, adapting it to the target. If you're recording in stereo, it will handle a stream of mixed mono and stereo fine. Opus never sees a change in the number of tracks encoded (which is the point of this function).
> My first thought is to follow the spec. If the spec doesn't mention what to
> do, and what to do isn't obvious, we should raise an error to show that the
> feature is unsupported and no further thought has been given on the
> behavior. Like we did for video resolution changes until we added support
> and tests.
This patch is just fixing a bug in the adaptive mixing to the target number of channels (which is what I think most people would expect; erroring on channels changing is not something I would anticipate as an app developer). And it already works for the mono input to a stereo recorder. (And probably usually works in the non-ASAN case, perhaps always or virtually so.)
> IMO compat is secondary here since it's not in the spec, and it's not a
> regression.
Bryce: when landing this bug, make the test a separate patch, and do NOT check in the test with the patch unless told to by the security guys (who give security-approval)
Group: media-core-security
Updated•7 years ago
|
Keywords: csectype-bounds,
sec-high
Updated•7 years ago
|
Summary: heap overflow in InterleaveTrackData → Write past end of allocation in InterleaveTrackData
Comment 12•7 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #11)
> (In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #9)
> > Is there even existing code to upmix and downmix? I see that we support mono
> > and stereo only, but nothing about dynamic changes. Is that even supported
> > by opus? If we want to support this we IMHO need a set of gtests to prove
> > its functionality.
>
> For audio playback and other reasons, we have dynamic up/down-mixers,
> including this one.
Ah, I see now. My apologies, this is not code that I know so well. Get a test up and I or jesup can r+.
Assignee | ||
Comment 13•7 years ago
|
||
Assignee | ||
Comment 14•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8872791 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8872793 -
Flags: review?(rjesup)
Comment 15•7 years ago
|
||
Regression from bug 901633 IIUC.
Blocks: 901633
status-firefox53:
--- → wontfix
status-firefox54:
--- → affected
status-firefox55:
--- → affected
status-firefox-esr45:
--- → wontfix
status-firefox-esr52:
--- → affected
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
tracking-firefox-esr52:
--- → ?
Version: unspecified → 43 Branch
Comment 16•7 years ago
|
||
Comment on attachment 8872793 [details] [diff] [review]
Add crashtest for media recorder when reducing number of source stream channels
Review of attachment 8872793 [details] [diff] [review]:
-----------------------------------------------------------------
Don't check in until told to do so by security (abillings/dveditz/etc)
Attachment #8872793 -
Flags: review?(rjesup) → review+
Updated•7 years ago
|
Attachment #8872553 -
Flags: review+
Comment 17•7 years ago
|
||
Bryce - request s-a and uplift at the same time, please.
Flags: needinfo?(bvandyk)
Assignee | ||
Comment 18•7 years ago
|
||
Comment on attachment 8872553 [details] [diff] [review]
Make sure number of channels is correctly set for opus encoder
Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:
Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:
Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined:
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:
See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
- An attacker familiar with the media recorder and processing APIs could likely figure out how to begin exploiting this. Once an attacker has figured out that this can be done, triggering the the read off the end of the array is trivial (see the PoC or the attached test). However, the data will be fed through the opus encoder before being exposed to the attacker, which increases the difficulty of exploiting this.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- The comment is left intentionally vague to avoid this. The code change very obviously deals with array indexing, one could likely infer that it is to avoid indexing off the end of an array. The follow up test should not be landed with this, as it paints a clear target by being a crash test. I will follow up with another bug to land it later.
Which older supported branches are affected by this flaw?
- All supported: 54, 55, and 52esr appear to be affected, as well as various older, unsupported branches.
If not all supported branches, which bug introduced the flaw?
- All supported branches appear to be impacted.
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
- I believe this patch should work for the other branches. This code doesn't look to have changed since 52[1], so the same patch should work.
How likely is this patch to cause regressions; how much testing does it need?
- The patch is small and specific, I do not think it likely it will cause regressions. It does slightly alter existing behaviour if users are non-maliciously changing channel counts (specifically if users create a mono recorder then change the input to stereo). However, I think questions about up and down mixing can be handled in a non secure follow up once this is seen to. Testing would be good to ensure this works, for security purposes decreasing the number of input channels appears to be the most important test. I don't think increasing the number of channels would result in the issue, but would be good to test also.
[1]: https://dxr.mozilla.org/mozilla-esr52/source/dom/media/encoder/TrackEncoder.cpp
Flags: needinfo?(bvandyk)
Attachment #8872553 -
Flags: sec-approval?
Attachment #8872553 -
Flags: approval-mozilla-esr52?
Attachment #8872553 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 19•7 years ago
|
||
Ack, follow up to previous comment:
Approval Request Comment
[Feature/Bug causing the regression]: Appear to be bug 901633 based on comment #15.
[User impact if declined]: Possible exposure of memory.
[Is this code covered by automated tests?]: Not yet, a test is proposed but should not be landed due to painting a target.
[Has the fix been verified in Nightly?]: This code has not yet been landed. Locally the fix works.
[Needs manual test from QE? If yes, steps to reproduce]: Probably good to test. Can be tested using the attached PoC.
[List of other uplifts needed for the feature/fix]: Only the proposed patch is required at this stage.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: It is a small and specific change.
[String changes made/needed]: None.
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Rating to be decided. At this stage uplift request is being made on a security basis.
User impact if declined: Possible exposure of memory.
Fix Landed on Version: Not yet landed. Would be 55.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None.
Comment 20•7 years ago
|
||
Release management, do we still have time to take this in 54?
If so, I will give sec-approval+ for trunk and you can give branch approvals.
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
Comment 21•7 years ago
|
||
Yes, we can still take it for 54.
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
Updated•7 years ago
|
Comment 22•7 years ago
|
||
Comment on attachment 8872553 [details] [diff] [review]
Make sure number of channels is correctly set for opus encoder
All approvals given.
Attachment #8872553 -
Flags: sec-approval?
Attachment #8872553 -
Flags: sec-approval+
Attachment #8872553 -
Flags: approval-mozilla-esr52?
Attachment #8872553 -
Flags: approval-mozilla-esr52+
Attachment #8872553 -
Flags: approval-mozilla-beta?
Attachment #8872553 -
Flags: approval-mozilla-beta+
Updated•7 years ago
|
Keywords: checkin-needed
Comment 23•7 years ago
|
||
Keywords: checkin-needed
Comment 24•7 years ago
|
||
Backed out for bustage.
https://hg.mozilla.org/integration/autoland/rev/15e32469eb048957223bac458140776a30251c6b
https://treeherder.mozilla.org/logviewer.html#?job_id=103769109&repo=autoland
Bruce or Randell, we're on a pretty tight time window here, so can one of you fix and re-land ASAP?
Flags: needinfo?(rjesup)
Flags: needinfo?(bvandyk)
Comment 25•7 years ago
|
||
Flags: needinfo?(rjesup)
Comment 26•7 years ago
|
||
uplift |
Flags: needinfo?(bvandyk)
Comment 27•7 years ago
|
||
uplift |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
Updated•7 years ago
|
Group: media-core-security, core-security → core-security-release
Updated•7 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•7 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main54+][adv-esr52.2+]
Updated•7 years ago
|
Alias: CVE-2017-7758
Comment 29•7 years ago
|
||
I've managed to reproduce the crash on 52.0.2 (Ubuntu 16.04 x64) and 53.0a1 Nightly (Mac OS X 10.11.6).
The crash is not reproducible anymore with 54.0 (20170605204906) and latest Nightly 55.a01 (2017-06-07) under the following OSes:
- Win 10 x64
- Mac OS 10.11.6
- Ubuntu 16.04 x64 LTS
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment 30•7 years ago
|
||
This is also verified on esr 52.2.0 (20170607123825).
Assignee | ||
Comment 31•7 years ago
|
||
Comment on attachment 8872793 [details] [diff] [review]
Add crashtest for media recorder when reducing number of source stream channels
Requesting sec approval to land delayed test case:
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
- This test makes it clear how the code can trigger the abusable bug. Weaponising the bug would require further effort.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- This is test that painted a target to a patched bug. Landing it has been delayed to avoid this.
Which older supported branches are affected by this flaw?
- All supported branches should be immune due to earlier patch.
If not all supported branches, which bug introduced the flaw?
- All supported branches should be immune.
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
- Backports are not needed.
How likely is this patch to cause regressions; how much testing does it need?
- Regressions in any other code are unlikely, any problems should be limited to the test itself.
Attachment #8872793 -
Flags: sec-approval?
Comment 32•7 years ago
|
||
Comment on attachment 8872793 [details] [diff] [review]
Add crashtest for media recorder when reducing number of source stream channels
Fixed in 54? Sure, land away!
Attachment #8872793 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 33•7 years ago
|
||
Assignee | ||
Comment 34•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc2cbce028cbdbe33bbbddaa2fd0213479fb3c2a
Bug 1368490 - Add crashtest for media recorder when reducing number of source stream channels. r=jesup
Comment 35•7 years ago
|
||
Flags: in-testsuite+
Updated•7 years ago
|
Group: core-security-release
Updated•6 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•