1372509 - (CVE-2017-7799) Self-XSS XUL Injection in about:webrtc

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P1)

Description

[Frederik Braun [:freddy]]

(This would be a sec-critical, if there was a way for an attacker to inject markup to this page. It seems to me the information comes from the browser user through actual interaction and it's unlikely someone creates and uses a file called <textarea> :-))


JavaScript in about:webrtc should use textContent or escape HTML before assigning to .innerHTML


1) 
http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/toolkit/content/aboutwebrtc/aboutWebrtc.js#143
>      this.msg.innerHTML =
>         `<span class="info-label">${this._messageHeader}:</span> ${this._message}`;

In this case, I suggest replacing ><'& with their respective entities for both variables. This could be done with a tagged template string and an escape function like https://github.com/fxos-eng/sanitizer/blob/master/sanitizer.js#L46-L57



In
http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/toolkit/content/aboutwebrtc/aboutWebrtc.js#308
>       msg.innerHTML = `${data.error.name}: ${data.error.message}`;
I suggest using textContent instead of innerHTML



I am assigning this to you jib, because you have been modifying this file in the past. Please help me find someone else if you can not take this bug.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Fix eslint warnings in about:webrtc

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8885442 [details] [diff] [review]
Fix eslint warnings in about:webrtc

Review of attachment 8885442 [details] [diff] [review]:
-----------------------------------------------------------------

Don't know a lot about eslint, but looks ok

Comment 3

[Frederik Braun [:freddy]]

Looks great, thank you!
(NB, I haven't seen this 'Object.assign(document.createElement(), ..)' before. That's pretty neat!)

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks, yeah Object.assign() is a good feature. If someone could push this for me that'd be great.

Comment 5

[Carsten Book [:Tomcat]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/2a6a73d9a560e954696c3c548e0a99c28e86cb34

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2a6a73d9a560

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Seems worth a backport request for Beta at least. Not sure if it's worth the churn for ESR52 since the attack seems mostly hypothetical?

Comment 8

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Agree.

Comment 9

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8885442 [details] [diff] [review]
Fix eslint warnings in about:webrtc

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1100508
[User impact if declined]: sec-moderate - see comment 0
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]:
Go to about:webrtc, click all three buttons (Save page requires file dialog confirm) and verify each adds one status line of text below.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]:
Trivial change. Small risk area (extremely small sub-pool of users visit about:webrtc)
[String changes made/needed]: None

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 8885442 [details] [diff] [review]
Fix eslint warnings in about:webrtc

input sanitization in about:webrtc, beta55+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b37c31641823

Comment 12

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to test the issue on Firefox 55.0b12 and on Firefox 56.0a1 (2017-07-26), under Windows 10x64, Ubuntu 16.04x64 and under Mac OS X 10.11 and the status lines are correctly displayed.

I've also tried to reproduce the issue on Firefox 52.2.1, Firefox 54.0.1 and on Firefox 56.0a1 (2017-06-12), under Windows 10x64, without success. The status lines (Save page, Debug Mode and AEC Logging) are also displayed on affected builds. 
Is this behavior expected?

Comment 13

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to test the issue on Firefox 55.0b12 and on Firefox 56.0a1 (2017-07-26), under Windows 10x64, Ubuntu 16.04x64 and under Mac OS X 10.11 and the status lines are correctly displayed.

I've also tried to reproduce the issue on Firefox 52.2.1, Firefox 54.0.1 and on Firefox 56.0a1 (2017-06-12), under Windows 10x64, without success. The status lines (Save page, Debug Mode and AEC Logging) are also displayed on affected builds. 
Is this behavior expected?

Comment 14

[Mihai Boldan, Desktop QA [:mboldan]]

Sorry for the double Commend and ni.

Comment 15

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks!

Yes there should be no visible difference in how the status lines are displayed. It's just using safer web-tech to get it there.

Comment 16

[Mihai Boldan, Desktop QA [:mboldan]]

According to Comment 13 and Comment 15, I'm marking this issue Verified Fixed.