JS URLs in many dialogs run as chrome

VERIFIED FIXED in mozilla1.0

Status

()

Core
Security
VERIFIED FIXED
15 years ago
10 years ago

People

(Reporter: Mitchell Stoltz (not reading bugmail), Assigned: Mitchell Stoltz (not reading bugmail))

Tracking

Trunk
mozilla1.0
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

See also bugs 108399 and 88143. Here's yet another one: viewing Page Info on a
page containing <object data="javascript:....."></object> can run chrome.

Updated

15 years ago
Depends on: 88143, 108399
Is there a way for a js dialog to be able to run with the privilages of a
particular url?
It is also bad that this works with javascript disabled.
(Assignee)

Comment 3

15 years ago
Critical Moz1.0 fixes
Status: NEW → ASSIGNED
Keywords: nsbeta1
Target Milestone: --- → mozilla1.0
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Comment 4

15 years ago
Created attachment 76135 [details] [diff] [review]
Patch for Element Properties dialog
(Assignee)

Updated

15 years ago
Keywords: patch
This is ugly. Is there an API for dialogs to drop privs for tthe current stack
frame + descendants?

This patch won't help with bug 127702, for example. Also, its not extendable to
other url schemes. Also, do you need to call checkloaduri or relatives?
(Assignee)

Comment 6

15 years ago
> This is ugly. Is there an API for dialogs to drop privs for tthe current stack
> frame + descendants?
No, there isn't such an API, and to attempt that at this point would be both
uglier and more regression-prone. I'm trying to tread lightly at this stage in
the game. This patch fixes all versions of the Element Properties dialog
(images, links, etc.).

> This patch won't help with bug 127702, for example. 
This patch was never intended to fix bug 127702. The fix for that bug will be
handled in nsImapURL.

> Also, its not extendable to other url schemes.
Sure it is. All we have to do is add those schemes to the regexp. In any case,
it's much safer to list the schemes we allow than the schemes we deny. I really
don't think it matters if links to MyWeirdNewProtocolHandler: URLs aren't live
in the properties dialog - that's really not a big deal.

> Also, do you need to call checkloaduri or relatives?
Yes, you're probably right about that. I'll see about adding CheckLoadURI.

We can always revisit this after Moz1.0, but for now I want a quick, safe,
uncomplicated fix.

Comment 7

15 years ago
Mitch, r=jrgm for the patch given (but convert the tabs in the patch to 
spaces).

But, [a] from your last comment it sounds like you may have another patch 
pending, and [b] I can't read the other bugs noted on this page (restricted
access), so maybe an r= from someone with more knowledge of the security apis
might be better (bbaetz?)
I agree with mstoltz that at the moment any patch which prevents the exploits is
much better than nothing.
(Assignee)

Comment 9

15 years ago
Created attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

This is the same patch plus a call to CheckLoadURI, which we should have been
doing here anyway.
Attachment #76135 - Attachment is obsolete: true

Updated

15 years ago
Attachment #76652 - Flags: review+

Comment 10

15 years ago
Comment on attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

r=jrgm, but fix the indentation (no tabs)
Yeah, this is fine, but please do file a bug on the alternative cleaner
solution, unless you don't think that such a method would be useful in teh
general case.
Comment on attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

sr=jst if you fix the bad indentation :-)
Attachment #76652 - Flags: superreview+

Comment 13

15 years ago
Comment on attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76652 - Flags: approval+
(Assignee)

Comment 14

15 years ago
The fix is in.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 15

15 years ago
Marking verified as per above developer comments.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

15 years ago
Group: security?

Updated

11 years ago
Flags: testcase+

Updated

10 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.