Closed
Bug 1321420
Opened 8 years ago
Closed 8 years ago
Add a pref to restrict content in browser requests
Categories
(Chat Core :: Twitter, defect)
Chat Core
Twitter
Tracking
(Not tracked)
RESOLVED
FIXED
Instantbird 53
People
(Reporter: arlolra, Assigned: arlolra)
Details
Attachments
(1 file, 2 obsolete files)
2.87 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.98 Safari/537.36
Attachment #8815941 -
Attachment is patch: true
Attachment #8815941 -
Attachment mime type: text/x-patch → text/plain
Comment 1•8 years ago
|
||
arlolra: Please include a description when filing bugs. There's no context about what this is trying to fix or what the issue is. Why was this change made? What is it fixing?
Flags: needinfo?(arlolra)
Updated•8 years ago
|
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Sorry for not providing context. This was in response to yesterday's exploit, https://blog.mozilla.org/security/2016/11/30/fixing-an-svg-animation-vulnerability/ I was trying to reduce the surface in which arbitrary JS can be executed in Instantbird.
Flags: needinfo?(arlolra)
Comment 3•8 years ago
|
||
Doesn't the twitter auth page rely on JS?
> Doesn't the twitter auth page rely on JS?
Unless I did something wrong, I built with this patch and was able to successfully authenticate. It's possible certain client side validation would be disabled, but that doesn't prevent it from working.
Summary: Restrict content in Twitter's auth browser → Add a pref to restrict content in browser requests
Attachment #8815941 -
Attachment is obsolete: true
Attachment #8816233 -
Flags: review?(aleth)
Comment 6•8 years ago
|
||
Comment on attachment 8816233 [details] [diff] [review] 0001-Bug-1321420-Add-a-pref-to-restrict-content-in-browse.patch Review of attachment 8816233 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/content/browserRequest.js @@ +137,5 @@ > let browser = document.getElementById("requestFrame"); > + > + if (Services.prefs.getBoolPref("chat.browserRequest.restrictContent")) { > + browser.docShell.allowJavascript = false; > + browser.docShell.allowPlugins = false; Do you need this? Aren't plugins disabled app-wide already?
Comment on attachment 8816233 [details] [diff] [review] 0001-Bug-1321420-Add-a-pref-to-restrict-content-in-browse.patch Review of attachment 8816233 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/content/browserRequest.js @@ +137,5 @@ > let browser = document.getElementById("requestFrame"); > + > + if (Services.prefs.getBoolPref("chat.browserRequest.restrictContent")) { > + browser.docShell.allowJavascript = false; > + browser.docShell.allowPlugins = false; https://github.com/mozilla/releases-comm-central/blob/master/im/app/profile/all-instantbird.js#L264-L272 I'm not sure what plugins in xpis are, but they seem to be enabled by default.
Comment 8•8 years ago
|
||
(In reply to arlolra from comment #7) > I'm not sure what plugins in xpis are, but they seem to be enabled by > default. Plugins in addons. I think we'd probably accept a patch to disable these (r? flo).
> I think we'd probably accept a patch to disable these
Happy to supply it, if desired.
Can we leave `allowPlugins = false` as defence in depth?
Comment 10•8 years ago
|
||
Comment on attachment 8816233 [details] [diff] [review] 0001-Bug-1321420-Add-a-pref-to-restrict-content-in-browse.patch Review of attachment 8816233 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/chat-prefs.js @@ +90,5 @@ > pref("chat.prpls.prpl-yahoo.disable", true); > // Whether to disable SRV lookups that use the system DNS library. > pref("chat.dns.srv.disable", false); > +// Restricting what documents that contain untrusted data can do. > +pref("chat.browserRequest.restrictContent", false); chat.browserRequest.disableJavascript would be clearer. ::: chat/content/browserRequest.js @@ +137,5 @@ > let browser = document.getElementById("requestFrame"); > + > + if (Services.prefs.getBoolPref("chat.browserRequest.restrictContent")) { > + browser.docShell.allowJavascript = false; > + browser.docShell.allowPlugins = false; If you really want to keep this, move it outside the if clause, I see no use case for plugins in chat browserRequests ;)
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8816233 -
Attachment is obsolete: true
Attachment #8816233 -
Flags: review?(aleth)
Attachment #8816291 -
Flags: review?(aleth)
Updated•8 years ago
|
Attachment #8816291 -
Flags: review?(aleth) → review+
Updated•8 years ago
|
Keywords: checkin-needed
Comment 12•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/a1ff30b2483661bb2eede5649de61b5501e3063f
Assignee: nobody → arlolra
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago → 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 53
You need to log in
before you can comment on or make changes to this bug.
Description
•