Closed Bug 152701 Opened 22 years ago Closed 22 years ago

More eavesdropping in mailnews...

Categories

(Core :: Security, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.2beta

People

(Reporter: jst, Assigned: security-bugs)

References

Details

(Whiteboard: [ADT2 RTM][sg:blocker])

Attachments

(1 file, 1 obsolete file)

Same problem that is described in bug 66938 only this time XMLSerializer can be used to turn a DOM into a string. Same approach applies for this problem, we just need to write a patch and check it in...
Keywords: nsbeta1+
Whiteboard: [ADT2 RTM]
Target Milestone: --- → mozilla1.0.1
What properties/functions do we need to block in mail messages?
Enabling javascript in mail and news is quite dangerous for probably more reasons than just sniffing the replies/forwards.
In addition to the properties we already blocked in bug 66938 we need to block all access to XMLSerializer, i.e. we don't want to even let people create XMLSerializer objects so we should block access to window.XMLSerializer, and XMLSerializer.* Hmm, what about XMLHttpRequest? Heikki, can that also be used to steal data?
Hmm, not sure... send() can take a document as an argument, but you should be able to open() only to same host. But if you can only open to same host, I don't see what good you could do with a mail or news server, so I am fine with disabling XMLHttpRequest in Mail as well. Btw, JS is/will be disabled in Mail by default so this thing is only to protect those users who enable it themselves...
removing (+) for re-nomination.
Keywords: nsbeta1+nsbeta1
Target Milestone: mozilla1.0.1 → mozilla1.1beta
Johnny, didn't we fix this already?
No, this bug is about blocking Mail from using XMLSerializer at all. I don't think we've done this. We should figure out some way to whitelist the functions available to scripts in mail without having to list hundreds of properties in prefs.
Status: NEW → ASSIGNED
I don't know that this was fixed yet. I.e. AFAIC this was *not* fixed yet.
Target Milestone: mozilla1.1beta → mozilla1.2beta
What if we turn completely off JS in the message display pane, but allow authors to include IFRAMEs in which JS will run (subject to checking the 'allow JS in mail/news' pref)?
Attached patch Potential fix (obsolete) (deleted) — Splinter Review
I am wondering if this would be the fix, haven't tested yet but will do asap.
Comment on attachment 100942 [details] [diff] [review] Potential fix sr=jst
Attachment #100942 - Flags: superreview+
Comment on attachment 100942 [details] [diff] [review] Potential fix This did not work :( When I changed the policies for the leaf properties then it worked (for example XMLSerializer.serializeToString). Shouldn't there be a simpler way to "noAccess" the whole object and all its methods?
Attachment #100942 - Attachment is obsolete: true
Whiteboard: [ADT2 RTM] → [ADT2 RTM][sg:blocker]
Blocks: 84545
We discussed this with mstoltz, and as far as we know there is currently no wildcard support for property and all its subproperties. That, in addition to what I have attached here, would be the reasonable solution. A last resort solution in the short term would be to list all the leaf properties in the all.js file, but there would need to be around 50 lines (rough estimate) added to all.js. The file is currently 783 lines so maybe 50 or so new lines wouldn't make a noticeable different in startup. I'll write this last resort patch as soon as I have some time from meetings. Please note that this approach is also fragile in the sense that adding/changing properties will make them accessible.
Ray, Harish: did I catch all properties? What about listeners? Also, if this is the approach we take, every time we change the XMLExtras/SOAP interfaces the changes need to be reflected here. The patch does not include WSDL changes because it is not built by default; when that happens those changes would need to be added.
Comment on attachment 102980 [details] [diff] [review] all.js fix What about nsIOnReadystatechangeHandler's handleEvent? Isn't that scriptable too?
From JS that property is 'undefined'.
Comment on attachment 102980 [details] [diff] [review] all.js fix Heikki found out that handleEvent is not scriptable and hence r=harishd.
Attachment #102980 - Flags: review+
This is on the 1.2 list. How is it coming along?
Comment on attachment 102980 [details] [diff] [review] all.js fix sr=dveditz Is the 1.0 branch affected by this bug?
Attachment #102980 - Flags: superreview+
Yes. I don't see this as high priority for the branch since JS is disabled by default in mail, but this seems like a safe fix if it is wanted.
It's not that serious a problem now that JS is off by default in mail, but the fix is easy and safe so it might be good to take on the branch. nominating. (does having all these caps entries slow us down?)
Keywords: adt1.0.2
I don't have any performance data to back this up, but I think the slowdown is unnoticeable. I have emailed ADT and drivers for approvals to trunk & branch.
Not vulnerable by default, adt- per triage meeting
Keywords: adt1.0.2adt1.0.2-
Comment on attachment 102980 [details] [diff] [review] all.js fix This is good for the 1.0 branch too -- a=brendan@mozilla.org on behalf of drivers. /be
Fixed on trunk & branch.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Keywords: fixed1.0.2
Resolution: --- → FIXED
Verified on 2002-11-20-branch on Win 2000. Ran the test cases under http://slip.mcom.com/projects/security/XMLExtras-Acceptance/xmlextras-tests.html and the test cases passed.
Status: RESOLVED → VERIFIED
Group: security
Summary: More eavsdropping in mailnews... → More eavesdropping in mailnews...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: