Closed Bug 123383 Opened 18 years ago Closed 18 years ago
JS URLs in many dialogs run as chrome
Is there a way for a js dialog to be able to run with the privilages of a particular url?
Critical Moz1.0 fixes
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
18 years ago
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?
> 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.
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.
This is the same patch plus a call to CheckLoadURI, which we should have been doing here anyway.
Attachment #76135 - Attachment is obsolete: true
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 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+
The fix is in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Marking verified as per above developer comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.