Closed Bug 1482659 (CVE-2018-18512) Opened 7 years ago Closed 6 years ago

startup crash in soundInitWavHdr playing sound file for new mail

Categories

(Core :: Widget: Win32, defect)

x86
Windows
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
thunderbird_esr60 + fixed
firefox-esr60 65+ fixed
firefox63 --- wontfix
firefox64 + wontfix
firefox65 + fixed
firefox66 + fixed

People

(Reporter: wsmwk, Assigned: tnguyen)

References

()

Details

(7 keywords, Whiteboard: [startupcrash][tbird topcrash][regression: TB60][post-critsmash-triage][adv-esr60.5+])

Crash Data

Attachments

(2 files)

For reasons unknown, this is has gone from obsurity in 52.x (almost zero crashes), to #3 crash for 60.0 - visually seen in the graph https://crash-stats.mozilla.com/signature/?product=Thunderbird&signature=soundInitWavHdr&date=%3E%3D2018-05-11T10%3A52%3A28.000Z&date=%3C2018-08-11T10%3A52%3A28.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#summary which also shows this as a startup crash This bug was filed from the Socorro interface and is report bp-19d63b14-ab22-4b0d-ad96-156eb0180808. ============================================================= Top 10 frames of crashing thread: 0 winmm.dll soundInitWavHdr 1 winmm.dll soundLoadMemory 2 winmm.dll _EH_prolog3_catch_GS 3 winmm.dll mmWndProc 4 user32.dll _InternalCallWinProc 5 user32.dll UserCallWinProcCheckWow 6 user32.dll DispatchMessageWorker 7 user32.dll DispatchMessageA 8 winmm.dll mciwindow 9 kernel32.dll BaseThreadInitThunk =============================================================
I haven't checked to see whether this is seen in nightly builds
90% of crashes version 60. Not a single nightly crash. 60% windows 10. Average user who crashes, has this crash sig about 6 times according to https://crash-stats.mozilla.com/topcrashers/?product=Thunderbird&version=60.0 Uptick started ~March 19 https://crash-stats.mozilla.com/signature/?signature=soundInitWavHdr&date=%3E%3D2017-12-01T10%3A21%3A00.000Z&date=%3C2018-03-28T13%3A21%3A00.000Z#graphs initially affecting 59.0b2 bp-1a5b803d-25eb-4df8-83cc-7a3dd0180316 2018-03-16 11:36:04 59.0b2 20180228045324 bp-0e1f244b-11e3-4e71-9775-875f90180317 2018-03-17 15:03:25 59.0b2 20180228045324 bp-9e713b75-8497-4994-aa73-9e8eb0180318 2018-03-18 20:19:27 59.0b2 20180228045324 bp-6d5aa79d-dcc5-441d-a7e3-d17f90180319 2018-03-19 02:37:15 59.0b2 20180228045324 bp-e6876d1a-ed15-4f77-8c02-16fe60180319 2018-03-19 11:48:30 59.0b2 20180228045324 bp-40d26fe7-6b61-4e72-93c3-0be460180319 2018-03-19 12:41:01 59.0b2 20180228045324 bp-5ebdf56a-1bd1-4923-aeb2-b23ee0180319 2018-03-19 15:23:58 59.0b2 20180228045324 bp-707bb4f2-62de-42d0-abbc-8a5a30180319 2018-03-19 17:10:25 59.0b2 20180228045324
Flags: needinfo?(m_kato)
(In reply to Wayne Mery (:wsmwk) from comment #2) > 90% of crashes version 60. Not a single nightly crash. > 60% windows 10. > Average user who crashes, has this crash sig about 6 times according to > https://crash-stats.mozilla.com/topcrashers/?product=Thunderbird&version=60.0 It seems to be buffer overrun since all address is over page boundary. > Uptick started ~March 19 > https://crash-stats.mozilla.com/signature/ > ?signature=soundInitWavHdr&date=%3E%3D2017-12-01T10%3A21%3A00. > 000Z&date=%3C2018-03-28T13%3A21%3A00.000Z#graphs > initially affecting 59.0b2 If this is generic issue, it will occur on Firefox, but no crash on Firefox. So I think that this depends on sound file (wav?) . Does Thunderbird (or c-c-) change sound find such as mail biff?
Flags: needinfo?(m_kato)
Right there are no FIrefox crashes and we do you sound for biff. I'd assume the cause to be Windows, except the strong Thunderbird connection plus it started. I was wrong about no nightly crashes bp-9d206d81-a855-4cb3-b2c9-0d3420180112 59.0a1 bp-5bd2ea09-184d-44bf-b5bb-fac0d0180213 57.0a1
CWaveQueue::AddToTail bp-3ae7c5f7-6276-4bf1-851a-1852a0180821 is the same? #10 crash for 60.0, is all win7, and started in the 56 beta time period like bp-eede566f-5542-4a76-9857-44e740180822 CWaveHandle::AddHeader is #31 crash, all win10 - bp-0183b6c7-2e66-4a9d-895a-7cb0a0180823
Flags: needinfo?(m_kato)
(In reply to Wayne Mery (:wsmwk) from comment #5) > CWaveQueue::AddToTail bp-3ae7c5f7-6276-4bf1-851a-1852a0180821 is the same? > #10 crash for 60.0, is all win7, and started in the 56 beta time period like > bp-eede566f-5542-4a76-9857-44e740180822 > > CWaveHandle::AddHeader is #31 crash, all win10 - > bp-0183b6c7-2e66-4a9d-895a-7cb0a0180823 I think no. It seems that sound file is corrupted, heap allocation failure, or a bug of microsoft's code, but I don't know why.
Flags: needinfo?(m_kato)
See Also: → 1495799
The user crashing here https://support.mozilla.org/en-US/questions/1236959 determined that disabling Lightning stopped the crash. I am guessing the issue may be with playing chrome://calendar/content/sound.wav or the user chosen replacement.
(In reply to Matt from comment #7) > The user crashing here https://support.mozilla.org/en-US/questions/1236959 > determined that disabling Lightning stopped the crash. > > I am guessing the issue may be with playing > chrome://calendar/content/sound.wav or the user chosen replacement. @Wayne: You answered a similar question from a user in 2016 (https://support.mozilla.org/en-US/questions/1148612) and it turned out to be a custom sound set by a user. A user recently posted this crash on m.s.thunderbird as well (https://crash-stats.mozilla.com/report/index/2876b771-2d99-4179-85ef-d06ff0181015) and I pointed them to the article where the user solved their own issue by resetting to default sounds. Perhaps the type of .WAV file is bad or of a different spec? Just a hunch. Maybe the user could submit the .WAV file for comparison against the default .WAV files in TB?
(In reply to Arthur K. from comment #8) > ... > ... A user recently posted this crash on m.s.thunderbird as well > (https://crash-stats.mozilla.com/report/index/2876b771-2d99-4179-85ef- > d06ff0181015) and I pointed them to the article where the user solved their > own issue by resetting to default sounds. Perhaps the type of .WAV file is > bad or of a different spec? Just a hunch. Maybe the user could submit the > .WAV file for comparison against the default .WAV files in TB? THanks all the great details. Can you make that contact and get the user engaged here?
Flags: needinfo?(thee.chicago.wolf)
Also signature memcpy | soundInitWavHdr bp-e0749a90-1111-4b58-9e26-5c3980181105
Crash Signature: [@ soundInitWavHdr] → [@ soundInitWavHdr] [@ memcpy | soundInitWavHdr]
(In reply to Wayne Mery (:wsmwk) from comment #9) > (In reply to Arthur K. from comment #8) > > ... > > ... A user recently posted this crash on m.s.thunderbird as well > > (https://crash-stats.mozilla.com/report/index/2876b771-2d99-4179-85ef- > > d06ff0181015) and I pointed them to the article where the user solved their > > own issue by resetting to default sounds. Perhaps the type of .WAV file is > > bad or of a different spec? Just a hunch. Maybe the user could submit the > > .WAV file for comparison against the default .WAV files in TB? > > THanks all the great details. Can you make that contact and get the user > engaged here? Through the forums, the user reported they'd fixed their issue. I'll reach out to see if they are interested in follow up for this bug.
Flags: needinfo?(thee.chicago.wolf)
I can't seem to find the thread where the user was talking about his crash so I can't reach out to him.
I read in the support forums that it is a known issue that Thunderbird crashes on large wav files. Is that relevant here perhaps.
See Also: 1495799
Crash Signature: [@ soundInitWavHdr] [@ memcpy | soundInitWavHdr] → [@ soundInitWavHdr] [@ memcpy | soundInitWavHdr] [@ CWaveQueue::AddToTail ] [@ CWaveHandle::AddHeader ]
Summary: startup crash in soundInitWavHdr → startup crash in soundInitWavHdr playing sound file for new mail
Attached audio Hawaii 50 2010.wav
RMS' 5mb testcase from bug 1498494 tested in clean profile from tools > options > general > play TB55.0b2 doesn't crash TB56.0b1 does crash So this should be solvable Alice can you get a regression range from nightlyes
Flags: needinfo?(alice0775)
(In reply to Wayne Mery (:wsmwk) from comment #17) > Created attachment 9023834 [details] > Hawaii 50 2010.wav > > RMS' 5mb testcase from bug 1498494 tested in clean profile from tools > > options > general > play > TB55.0b2 doesn't crash > TB56.0b1 does crash > > So this should be solvable > > Alice can you get a regression range from nightlyes I think disabling(removing) the such cosmetic sub-feature is best way for maintenance purpose. Regression window: https://hg.mozilla.org/comm-central/pushloghtml?fromchange=2a185ec18e89412466841a9755c49561f05bf142&tochange=6ccad3aa817af34d04fd8585ba0ce642f0b9193b https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d50abca6521baeae8ac6b07ddf843d63a1aa5f84&tochange=8f80d594c08d5c7a112e5d4b9eb44ffca717eb7b
Flags: needinfo?(alice0775)
Ah, I think that SND_ASYNC causes this issue. During playing sound, data may be free. sound is play on non-main thread, we don't need SND_ASYNC flag. UAF... Previous fix calls ::PlaySound(nullptr, nullptr, SND_PURGE). So it waits sound is finished.
Group: core-security
Component: General → Widget: Win32
Product: Thunderbird → Core
This seems to be regression of bug 1363163.
Assignee: nobody → m_kato
Keywords: regression
Assignee: m_kato → nobody
tnguyen, could you handle this since this seems to be regression by your bug 1363163? I think that root cause is that you remove ::PlaySound(nullptr, nullptr, SND_PURGE) or you set SND_ASYNC to play sound. Thanks, Alice-san and Wayne.
Flags: needinfo?(tnguyen)
I can reproduce the crash on Nightly65.0a1, but not Firefox64.0b7 windows10. Steps to reproduce: 1. set accessibility.typeaheadfind = true 2. set accessibility.typeaheadfind.soundURL = file:///C:/hogehoge/Hawaii%2050%202010.wav 3. Open web page https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions 4. Key press ex Actual Results: Tab crashes, or browser crashes sometimes Expected Results: Not crash
(In reply to Alice0775 White from comment #24) > I can reproduce the crash on Nightly65.0a1, but not Firefox64.0b7 windows10. > > Steps to reproduce: > 1. set accessibility.typeaheadfind = true > 2. set accessibility.typeaheadfind.soundURL = > file:///C:/hogehoge/Hawaii%2050%202010.wav > 3. Open web page > https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions > 4. Key press ex > > Actual Results: > Tab crashes, or browser crashes sometimes > > Expected Results: > Not crash Regression window(autoland): https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2366aeabd64c6ea5c7723b8f6bbcf3d5c0c85d46&tochange=1e6c23942e9b95bb3d729e0b9f2d8fc0294c7549
After bug 1401678, nsISound will work well even if on content process, so it is easy to reproduce this.
OS: Windows 10 → Windows
Whiteboard: [startupcrash] → [startupcrash][tbird topcrash]
Thanks for reporting this bug. I could take a look, but please wait for a while, I don't have Windows platform at the moment
Flags: needinfo?(tnguyen)
Blocks: 1363163
Whiteboard: [startupcrash][tbird topcrash] → [startupcrash][tbird topcrash][regression: TB60]
Group: core-security → layout-core-security
Blocks: 1495799
Ethan, is there someone else on your team with access to a Windows machine able to take a look at this? Would be great if we could get this fixed during this cycle still.
Flags: needinfo?(ettseng)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #28) > Ethan, is there someone else on your team with access to a Windows machine > able to take a look at this? Would be great if we could get this fixed > during this cycle still. Thomas, please let me know if you have problem to debug on Windows platform.
Assignee: nobody → tnguyen
Flags: needinfo?(ettseng)
I will take a look at this, I have just loaned a Windows machine and got it. Thanks for assigning that to me :)
Finally, I had a good Windows device to debug this bug. At the first look (build with release config), I tried to remove all ::Playsound calls in https://searchfox.org/mozilla-central/source/widget/windows/nsSound.cpp including ::PlaySound(nullptr, nullptr, SND_PURGE) and PlaySoundW with SND_ASYNC And it still crashes follow steps in comment 24. I suspect that is not related to Playsound or we have 2 separate issue here. I will take a closer look
(In reply to Thomas Nguyen from comment #31) > Finally, I had a good Windows device to debug this bug. At the first look > (build with release config), I tried to remove all ::Playsound calls in > https://searchfox.org/mozilla-central/source/widget/windows/nsSound.cpp > > including ::PlaySound(nullptr, nullptr, SND_PURGE) and PlaySoundW with > SND_ASYNC > And it still crashes follow steps in comment 24. I suspect that is not > related to Playsound or we have 2 separate issue here. > I will take a closer look I am pretty sure the crash in steps comment 24 is not related to Playsound API (windows api), I followed the steps on Mac OSX and I had the same crashes.
(In reply to Thomas from comment #32) > I am pretty sure the crash in steps comment 24 is not related to Playsound API (windows api), > I followed the steps on Mac OSX and I had the same crashes. Thomas, was that with Thunderbird? Dossy tried "5.3 MB MP3 file and it didn't crash. This is on OSX High Sierra 10.13.6 on TB 60.3.2."
Flags: needinfo?(tnguyen)
I tried Firefox (steps comment 24). Firefox seems to work well with that API, and the crash in comment 24 should be caused by another reason. Probably we have to change this bug's flag to Thunderbird only.
Flags: needinfo?(tnguyen)
But you are right, flags |= SND_MEMORY | SND_ASYNC seems to be a potential issue.
Do we need a different set of eyes on this?
SND_MEMORY | SND_ASYNC flag could cause a very high chance of crashing. Data may be free, but it’s entirely possible that the PlaySound API is still using it.
Sorry for the delay, I could confirm SND_MEMORY | SND_ASYNC is our problem. Neither of them is dangerous until we combine the two together and there's a chance of crashing due to the combination of these flags. I removed the SND_ASYNC and the crash is fixed in thunderbird. We don't Hi Makoto, do you agree with the fix? The playsound API implementation is non-public and I have no idea to see what will happen with and w/o SND_ASYNC.
Attachment #9031434 - Flags: feedback?(m_kato)
Comment on attachment 9031434 [details] Bug 1482659 - Purge the last sound, free then start to play a sound Apparently reviewed? (I can't see the patch in phabricator)
Flags: needinfo?(tnguyen)
Attachment #9031434 - Flags: feedback?(m_kato)
Yes, the patch was r+. I am trying to run several times in firefox to see if there's any problem before I could land this
Flags: needinfo?(tnguyen)
I could not land the patch because it causes a regression. Removing SND_ASYNC makes the function could not return immediately. It may not block UI (running in a thread) but the sound is still playing. Currently Playsound API implementation is limited accessed and I don't know a good idea to stop playing sound in a thread without SND_ASYNC. masayuki, do you have any idea?
Flags: needinfo?(masayuki)
That's weird if Firefox and Thunderbird are using the same nsISound but the crashes are only in Thunderbird. There is no particular difference, except the call in Firefox https://searchfox.org/mozilla-central/rev/47edbd91c43db6229cf32d1fc4bae9b325b9e2d0/widget/windows/nsSound.cpp#226 I will try tomorrow.
Ah, I see... How about to keep grabbing nsSoundPlayer instance in the player thread even after calling Playsound with SND_ASYNC and SND_PURGE, and release it immediately after next call of Playsound? If it helps, you should release the last sound when the main process is being shut down for avoiding the memory leak detection.
Flags: needinfo?(masayuki)
We don't know when PlaySound API finish/stop playing, so we have to keep the sound data alive until the next sound is played. The sound data we will keep is totally useless after the sound player finishes, but I think that's what we have at the moment. Masayuki, could you please take a look at the update?
Flags: needinfo?(masayuki)
Flags: needinfo?(masayuki)
(I assume NI was cancelled to avoid spammage during the holiday)
Flags: needinfo?(masayuki)
Cleared ni because working in phabricator
Flags: needinfo?(masayuki)
Have someone a Workaround for me, because I need my calendar items very urgent. Can I go to an older version Thunderbird/Lightning? Whne yes, which Version is working and how can I activate an older Version? How can I export my Thunderbird data (Email, addresses, calendar items (without lightning), to switch a Microsoft Product? Thanks for the help
You can download an older version here and disable updates. http://ftp.mozilla.org/pub/thunderbird/releases/52.9.1/ You need to click through to the right platform and language. If you disable custom sounds with TB 52, TB 60 should then work on the new profile. Good luck with Microsoft!
Super, it's work now fine with TB 60 and Lightning. So I do not need to Switch to Microsoft. Thanks
Attachment #9031434 - Attachment description: Bug 1482659 Remove SND_ASYNC flag when playing sound → Bug 1482659 - Keep sound data alive until playing the next sound
Attachment #9031434 - Attachment description: Bug 1482659 - Keep sound data alive until playing the next sound → Bug 1482659 - Purge the last sound, free then start to play a sound

Comment on attachment 9031434 [details]
Bug 1482659 - Purge the last sound, free then start to play a sound

[Security Approval Request]

How easily could an exploit be constructed based on the patch?: Not easy to exploit in Firefox, but easy to test in Thunderbird. Thunderbird supports playing a sound directly when user click playing (preview) sound.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No

Which older supported branches are affected by this flaw?: firefox <= 65, firefox-esr60

If not all supported branches, which bug introduced the flaw?: Bug 1482659

Do you have backports for the affected branches?: Yes

If not, how different, hard to create, and risky will they be?:

How likely is this patch to cause regressions; how much testing does it need?: Not in Firefox. But it would be great if we could test in Thunderbird more because Thunderbird supports playing a sound directly.

Attachment #9031434 - Flags: sec-approval?

Comment on attachment 9031434 [details]
Bug 1482659 - Purge the last sound, free then start to play a sound

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1363163

User impact if declined: Sometimes crash when playing sound, particularly in thunderbird

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: - Go to Thunderbird Menu -> Options -> Options -> Generals

  • Change the custom sound to the sound with sizes > 5 Mb.
  • Play

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): We did not change to much, just keep the sound alive longer

String changes made/needed:

Attachment #9031434 - Flags: approval-mozilla-beta?

Comment on attachment 9031434 [details]
Bug 1482659 - Purge the last sound, free then start to play a sound

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration:

User impact if declined: Sometimes crash when playing sound, particularly in thunderbird

Fix Landed on Version:

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): We did not change too much, just keep the sound data alive longer

String or UUID changes made by this patch: No

Attachment #9031434 - Flags: approval-mozilla-esr60?

Comment on attachment 9031434 [details]
Bug 1482659 - Purge the last sound, free then start to play a sound

Approvals given. Please get this checked in ASAP so it can make upcoming beta builds.

Attachment #9031434 - Flags: sec-approval?
Attachment #9031434 - Flags: sec-approval+
Attachment #9031434 - Flags: approval-mozilla-esr60?
Attachment #9031434 - Flags: approval-mozilla-esr60+
Attachment #9031434 - Flags: approval-mozilla-beta?
Attachment #9031434 - Flags: approval-mozilla-beta+
Group: layout-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Flags: qe-verify+

I am not able to reproduce the initial issue on the affected Firefox builds, using the steps provided in comment 24, neither on Windows 10 x64 nor on Windows 7 x86.
Alice, can you please try to confirm the fix on the targeted versions?

Flags: needinfo?(alice0775)

I can still crash with str comment 24 on Nightly66.0a1.
Crash ID bp-2251238b-40f8-46f0-bd13-deac00190114
Build ID 20190114104248
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0

Flags: needinfo?(alice0775)

That is a different issue and not related to PlaySound API, library. Should be an easy fix, because it is an assertion that prevents the init from calling in content.
https://hg.mozilla.org/mozilla-central/annotate/3dc7d345da5282969b71079d483ff8c30558819d/widget/nsSoundProxy.cpp#l45
Could you please file another bug about that?

(In reply to Thomas Nguyen from comment #64)

That is a different issue and not related to PlaySound API, library. Should
be an easy fix, because it is an assertion that prevents the init from
calling in content.
https://hg.mozilla.org/mozilla-central/annotate/
3dc7d345da5282969b71079d483ff8c30558819d/widget/nsSoundProxy.cpp#l45
Could you please file another bug about that?

filed: Bug 1519882

Dropping the qe+ plus flag based on the previous comments.

Flags: qe-verify+
Flags: qe-verify-
Whiteboard: [startupcrash][tbird topcrash][regression: TB60] → [startupcrash][tbird topcrash][regression: TB60][post-critsmash-triage]

Wayne,
Does this reproduce in Firefox at all? It seems like, while the affecting code exists everywhere, this is a Thunderbird only issue as far as triggering. If so, I would ask TB people to write a security advisory text for it once TB ships a version with the fix but will not include it in the Firefox advisories.

Flags: needinfo?(vseerror)

kaie, can you supply the draft CVE? ping me on irc if any questions.

Flags: needinfo?(vseerror) → needinfo?(kaie)

Looking at the patch, I wonder if there could still be a race. Maybe I'm wrong, I've asked in phabricator to doublecheck.

Here's a potential CVE text. Please double check for correctness.

CVE-2019-NNNN:
title: Use-after-free during sound notifications
impact: critical
reporter:
description: |
crash when playing a sound notification. The memory storing the sound data was immediately freed, although the sound is still being played asynchronously, causing a use-after-free crash.
bugs:
- url: 1482659

Flags: needinfo?(kaie)

(In reply to Kai Engert (:kaie:) from comment #69)

Looking at the patch, I wonder if there could still be a race.

After learning more about recent C++ features, I take this back. The code seems fine.

Kai,

I don't recall this ever showing up in an advisory for Thunderbird. Should we write one now for the issue?

Whiteboard: [startupcrash][tbird topcrash][regression: TB60][post-critsmash-triage] → [startupcrash][tbird topcrash][regression: TB60][post-critsmash-triage][adv-esr60.5+]

(In reply to Al Billings [:abillings] from comment #71)

I don't recall this ever showing up in an advisory for Thunderbird. Should we write one now for the issue?

Al, yes, I think we're ready to do so. Comment 69 has draft CVE text.

Based on the commit from Jan 10, it should have been fixed starting with TB 60.5.0 released Jan 29.

Alias: CVE-2018-18512
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: