Last Comment Bug 123383 - JS URLs in many dialogs run as chrome
: JS URLs in many dialogs run as chrome
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.0
Assigned To: Mitchell Stoltz (not reading bugmail)
: bsharma
Mentors:
Depends on: 88143 108399
Blocks:
  Show dependency treegraph
 
Reported: 2002-02-04 12:19 PST by Mitchell Stoltz (not reading bugmail)
Modified: 2007-04-01 14:16 PDT (History)
7 users (show)
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for Element Properties dialog (931 bytes, patch)
2002-03-25 18:46 PST, Mitchell Stoltz (not reading bugmail)
no flags Details | Diff | Splinter Review
Patch v2 - includes CheckLoadURI call (1.50 KB, patch)
2002-03-28 15:25 PST, Mitchell Stoltz (not reading bugmail)
jrgmorrison: review+
jst: superreview+
asa: approval+
Details | Diff | Splinter Review

Description Mitchell Stoltz (not reading bugmail) 2002-02-04 12:19:54 PST
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.
Comment 1 Bradley Baetz (:bbaetz) 2002-02-04 17:17:20 PST
Is there a way for a js dialog to be able to run with the privilages of a
particular url?
Comment 2 georgi - hopefully not receiving bugspam 2002-02-05 09:10:35 PST
It is also bad that this works with javascript disabled.
Comment 3 Mitchell Stoltz (not reading bugmail) 2002-03-08 16:38:53 PST
Critical Moz1.0 fixes
Comment 4 Mitchell Stoltz (not reading bugmail) 2002-03-25 18:46:56 PST
Created attachment 76135 [details] [diff] [review]
Patch for Element Properties dialog
Comment 5 Bradley Baetz (:bbaetz) 2002-03-25 20:22:22 PST
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?
Comment 6 Mitchell Stoltz (not reading bugmail) 2002-03-26 10:05:49 PST
> 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 John Morrison 2002-03-26 17:26:16 PST
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?)
Comment 8 georgi - hopefully not receiving bugspam 2002-03-27 08:30:19 PST
I agree with mstoltz that at the moment any patch which prevents the exploits is
much better than nothing.
Comment 9 Mitchell Stoltz (not reading bugmail) 2002-03-28 15:25:08 PST
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.
Comment 10 John Morrison 2002-03-28 15:57:14 PST
Comment on attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

r=jrgm, but fix the indentation (no tabs)
Comment 11 Bradley Baetz (:bbaetz) 2002-03-29 04:07:41 PST
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 12 Johnny Stenback (:jst, jst@mozilla.com) 2002-03-29 14:27:35 PST
Comment on attachment 76652 [details] [diff] [review]
Patch v2 - includes CheckLoadURI call

sr=jst if you fix the bad indentation :-)
Comment 13 Asa Dotzler [:asa] 2002-03-29 14:52:14 PST
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
Comment 14 Mitchell Stoltz (not reading bugmail) 2002-04-01 11:06:12 PST
The fix is in.
Comment 15 bsharma 2002-04-03 09:53:37 PST
Marking verified as per above developer comments.

Note You need to log in before you can comment on or make changes to this bug.