Bug 1372509 (CVE-2017-7799)

Self-XSS XUL Injection in about:webrtc

VERIFIED FIXED in Firefox 55

Status

()

defect
P1
normal
Rank:
14
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: freddyb, Assigned: jib)

Tracking

({csectype-priv-escalation, sec-moderate, wsec-xss})

unspecified
mozilla56
Points:
---
Bug Flags:
sec-review +

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox54 wontfix, firefox55 verified, firefox56 verified)

Details

(Whiteboard: [adv-main55+][post-critsmash-triage])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
(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.
Rank: 14
Priority: -- → P1
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
Attachment #8885442 - Flags: review?(rjesup) → review+
(Reporter)

Comment 3

2 years ago
Looks great, thank you!
(NB, I haven't seen this 'Object.assign(document.createElement(), ..)' before. That's pretty neat!)
Flags: sec-review+
Thanks, yeah Object.assign() is a good feature. If someone could push this for me that'd be great.
https://hg.mozilla.org/mozilla-central/rev/2a6a73d9a560
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
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?
Flags: needinfo?(jib)
Agree.
Flags: needinfo?(jib)
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
Attachment #8885442 - Flags: approval-mozilla-beta?
Comment on attachment 8885442 [details] [diff] [review]
Fix eslint warnings in about:webrtc

input sanitization in about:webrtc, beta55+
Attachment #8885442 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Group: media-core-security → core-security-release
Flags: qe-verify+
Alias: CVE-2017-7799
Whiteboard: [adv-main55+]
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?
Flags: needinfo?(rjesup) needinfo?(jib)
Flags: needinfo?(rjesup)
Flags: needinfo?(jib)
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?
Sorry for the double Commend and ni.
Flags: needinfo?(rjesup)
Flags: needinfo?(jib)
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.
Flags: needinfo?(jib)
Whiteboard: [adv-main55+] → [adv-main55+][post-critsmash-triage]
According to Comment 13 and Comment 15, I'm marking this issue Verified Fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(rjesup)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.