1764778 - PSpeechSynthesis can be started by a compromised child process even with the pref disabled

Status: RESOLVED FIXED

(Core :: Web Speech, defect)

Description

[Andrew McCreight [:mccr8]]

The method ContentParent::AllocPSpeechSynthesisParent() is used to create a new SpeechSynthesisParent if the child process sends the parent process a message. However, this method does not check that speech synthesis is enabled. Therefore, if an attacker has achieved arbitrary code execution in a child process, then I think it can start up the speech synthesis IPDL protocol in the parent, exposing Firefox to potential issues with the protocol.

Comment 1

[Andrew McCreight [:mccr8]]

I ran the tests in dom/media/webspeech/synth/test/ and they passed, which is hopefully enough of a smoke test for this.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

Comment 3

[Andrew McCreight [:mccr8]]

I'm not sure what rating to give this bug. Maybe sec-want? I skimmed over the code in speech synthesis parent and it wasn't doing anything dodgy looking with raw pointers. I guess there is mTask::mActor but that does get cleaned up in the dtor. I'm not sure if a compromised content process could get the parent to call into whatever OS speech service is being used, or if there are other guards for that.

Comment 4

[Andrew McCreight [:mccr8]]

I'll mark this sec-moderate, but maybe that's too low.

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 9272756 [details]
Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: This is a sec-moderate, so technically it doesn't need approval, but I'm not entirely sure if that's right. To turn this into an exploit, somebody would need to gain arbitrary code execution in the child process, then find some other issue with the speech synthesizer in the parent process. I did some light auditing and didn't find anything that looked bad.
• 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?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should be trivial. This code hasn't changed much as far as I can see.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. The biggest risk would be breaking tests that use this feature.

Comment 6

[Tom Ritter [:tjr] (OOTO until mid-August)]

Comment on attachment 9272756 [details]
Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

Approved to land and uplift (which I think we should definitely do.)

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/c2edd297f2c30911151ce79920311e337147c15e
https://hg.mozilla.org/mozilla-central/rev/c2edd297f2c3

Comment 8

[Andrew McCreight [:mccr8]]

Comment on attachment 9272756 [details]
Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Probably not a severe security issue, but I didn't audit everything so I can't be entirely sure.
• User impact if declined: possible sec issues
• Fix Landed on Version: 101
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only changes code used in tests

Beta/Release Uplift Approval Request

• User impact if declined:
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed: none
• Is Android affected?: Yes

Comment 9

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9272756 [details]
Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

Approved for 100.0b9

Comment 10

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

https://hg.mozilla.org/releases/mozilla-beta/rev/9c3024e88044

Comment 11

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9272756 [details]
Bug 1764778 - Check media.webspeech.synth.enabled in AllocPSpeechSynthesisParent.

Approved for 91.9esr

Comment 12

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

https://hg.mozilla.org/releases/mozilla-esr91/rev/b2d407e7db43

Comment 13

[Frederik Braun [:freddy]]

Attachment: advisory.txt