Closed Bug 458883 Opened 16 years ago Closed 16 years ago

Make Document.documentURI and .textContent noAccess in mailnews

Categories

(MailNews Core :: Security, defect)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bzbarsky, Assigned: philor)

References

Details

(Keywords: verified1.8.1.18, verified1.9.0.4, Whiteboard: [sg:high])

Attachments

(4 files, 1 obsolete file)

See bug 453928 comment 5.

We need to fix this on the 1.8 branch and 1.9.0 branch no matter what we're doing on trunk.
Flags: blocking1.9.0.4?
Flags: blocking1.8.1.18?
Group: core-security
Boris, there are no shipping mailnews products from 1.9.0. Are you sure we need to fix it there?

CCing dmose to look at this (or assign it).
No idea.  I just flagged this as blocking? for all stable branches where this would be a problem.  If you don't ever plan to ship Thunderbird or Seamonkey from 1.9.0.x, then no need to worry about it there too much, but it might be better to just fix and not worry than try to guess.  It's a one-liner...

Oh, I'll happily r+sr the fix, of course.  ;)
It's actually a two-liner, since I realized this afternoon that we never blocked .textContent either, which means that we only stopped bug 66938 for a bit under two years, and have been completely open to it since June 2003.

And we want to take it for 1.9.0 because even though *we* aren't shipping from it, we don't actually know that, for instance, kreeger isn't going to ship a Correo from 1.9.0, or that someone we don't even know is already shipping from it. What's that thing mscott's been doing, and what Gecko does it ship?
Assignee: nobody → philringnalda
OS: Mac OS X → All
Hardware: PC → All
Ah, I'd assumed we prevented innerHTML because of getting resolved URIs or something.

If we're preventing textContent we probably also need to kill nsIDOM3Text.wholeText.

Times like this I really wish we had brendan's tainting stuff working...
And we prevent *.attributes.get, *.getAttribute, and *.getAttributeNS, but not getAttributeNode or getAttributeNodeNS, as long as we're tightening stuff up here.
Attached file test mbox
Be nice if we had Mochitest-alike infrastructure (well, and had it on 1.8, since we're not doing script on the trunk right now), but at least this makes ad-hoc testing a little easier: drop it in Mail/Local Folders in a Tb2.x profile and view the message with javascript.allow.mailnews enabled, and it'll alert the documentURI (including the path to your profile, for easier attacking), then the textContent of the body to demonstrate that bug 66938 has been wide open again since 2003.

(I do have a nice automatic wiretap message, that will send my web server everything that the first recipient says to the person they forward it to, along with the path to both their profiles, but I figured nobody would really want to test with that.)
Attached patch Whack-some-moles, v.1 (obsolete) — Splinter Review
Against 1.8, since that's what worries me most, this is what I want to land on mozilla-central, 1.9.0, and 1.8.

With wholeText being 1.9.1-only, and 1.9.1 not having any content JS in mailnews at the moment, I'll get it separately (hopefully along with a reordering that will make it clear what we're actually doing, so things like the missing getAttributeNode(NS) will stand out better).
Attachment #342203 - Flags: superreview?(bzbarsky)
Attachment #342203 - Flags: review?(bzbarsky)
Attachment #342203 - Flags: superreview?(bzbarsky)
Attachment #342203 - Flags: superreview+
Attachment #342203 - Flags: review?(bzbarsky)
Attachment #342203 - Flags: review+
Comment on attachment 342203 [details] [diff] [review]
Whack-some-moles, v.1

That should probably be *.documentURI, not HTMLDocument.documentURI.  Otherwise if we somehow enable creation of XML documents with the current URI the caller will be able to extract it.
Comment on attachment 342203 [details] [diff] [review]
Whack-some-moles, v.1

* it is, then - I wasn't sure how expensive they are.

Drivers: sorry, no place other than 1.8.1 it *can* bake.
Attachment #342203 - Flags: approval1.9.0.4?
Attachment #342203 - Flags: approval1.8.1.18?
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.4?
Flags: blocking1.9.0.4+
Flags: blocking1.8.1.18?
Flags: blocking1.8.1.18+
Whiteboard: [sg:high]
Comment on attachment 342203 [details] [diff] [review]
Whack-some-moles, v.1

>+pref("capability.policy.mailnews.HTMLDocument.documentURI", "noAccess");

Sorry to nitpick but can we get a patch with the "*" as you intend to check it in? Often times downstream vendors work from the bug patch rather than bonsai so it's important to approve what's actually being checked in on the old branches.
Attachment #342203 - Flags: approval1.9.0.4?
Attachment #342203 - Flags: approval1.9.0.4-
Attachment #342203 - Flags: approval1.8.1.18?
Attachment #342203 - Flags: approval1.8.1.18-
Attachment #342203 - Attachment is obsolete: true
Attachment #342539 - Flags: approval1.8.1.18?
Second verse, same as the first (other than 31 lines of offset, but to me you are describing possible users of it as both psychotic and utterly random, so maybe avoiding the offset is a good thing).
Attachment #342540 - Flags: approval1.9.0.4?
Attachment #342540 - Attachment is patch: true
Attachment #342540 - Attachment mime type: application/octet-stream → text/plain
Summary: Make Document.documentURI noAccess in mailnews → Make Document.documentURI and .textContent noAccess in mailnews
Comment on attachment 342539 [details] [diff] [review]
For checkin, 1.8 branch

Approved for 1.8.1.18, a=dveditz for release-drivers
Attachment #342539 - Flags: approval1.8.1.18? → approval1.8.1.18+
Comment on attachment 342540 [details] [diff] [review]
For checkin, 1.9.0 branch

Approved for 1.9.0.4, a=dveditz for release-drivers
Attachment #342540 - Flags: approval1.9.0.4? → approval1.9.0.4+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Any suggestions on how QA can verify this fix?
Comment 6 - feel free to email me if you need more explicit directions.
Ah, right, I was skimming too quickly and didn't process your comment (obviously). This should be fine for verification.
Verified for 1.8.1.18 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.18pre) Gecko/2008110403 Thunderbird/2.0.0.18pre. I reproduced the bug in 1.8.1.17 using Phil's test mbox.
Verified for 1.9.0.4 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081104 Shredder/3.0b1pre.
Status: RESOLVED → VERIFIED
Comment on attachment 342539 [details] [diff] [review]
For checkin, 1.8 branch

a=asac for 1.8.0 (needs a bit context adjusting to apply cleanly)
Attachment #342539 - Flags: approval1.8.0.15+
Group: core-security
Flags: blocking1.8.0.next+
You need to log in before you can comment on or make changes to this bug.