Closed
Bug 495176
Opened 16 years ago
Closed 16 years ago
[FIX]Permission denied message could include document.domain was set
Categories
(Core :: Security: CAPS, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: johnjbarton, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
|
15.25 KB,
patch
|
jst
:
review+
Pike
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
From the newsgroup post:
A colleague within IBM contacted me about the error message:
Permission denied to access property Window.document
His site was opening a new window from the same domain setting masterWindow to window.opener. Then the access masterWindow.twGroup fails with the subject message.
I tried to help without much luck. Eventually he checked out the Firefox source code, built a debug version, traced through the code and found that the problem was on line 941 of nsScriptSecurityManager.cpp.
// If both or neither explicitly set their domain, allow the access
if (subjectSetDomain == objectSetDomain)
return NS_OK;
/*
** Access tests failed, so now report error.
*/
return NS_ERROR_DOM_PROP_ACCESS_DENIED;
This test fails in his case. A comment slight above this pointed to bug 154930 which points to 183143. The fix is to set the following code in the opener:
document.domain = document.domain;
Just set aside the natural reaction to the absurd solution, that is not the point here. It's the absurd level of work needed to understand an error message that I want to bring to your attention.
Instead, imagine what the scenario would be like if the message returned was
"See Bug 183143"
Of course the chain of failed tests would be better:
Either <SubjectURI> or <objectURI> explicitly set document.domain;
The values of document.domain in the subject and object do not match.
but since the error propagation technology seems to be limited to integers, may be the bug number is better.
Even if this story does not convince anyone of the importance of error messages, it may prove useful to someone searching for the subject error message.
Boris' reply:
The message in current builds is:
Error: Permission denied for <http://whatever> to get property
Window.document from <http://whatever>.
Source File: http://whatever/test.html
Line: 4
which is not necessarily that much better, but makes it clear at least which pages you need to look at.
It wouldn't be all that hard to include whether document.domain was set in this message, for what it's worth.
---
See also:
Bug 183143 - Setting document.domain doesn't match an implicit parent domain
Bug 154930 - document.domain abused to access hosts behind firewall
| Assignee | ||
Updated•16 years ago
|
Assignee: nobody → dveditz
Component: General → Security: CAPS
QA Contact: general → caps
Summary: Permission denied message could include document.domain was set → [FIX]Permission denied message could include document.domain was set
| Assignee | ||
Comment 1•16 years ago
|
||
Examples of error output with that patch:
JavaScript error: http://web.mit.edu/bzbarsky/www/foo.html, line 4: Permission denied for <http://web.mit.edu> (document.domain=<http://web.mit.edu>) to get property Window.document from <http://web.mit.edu> (document.domain has not been set).
JavaScript error: http://web.mit.edu/bzbarsky/www/foo.html, line 4: Permission denied for <http://web.mit.edu> (document.domain has not been set) to get property Window.document from <http://web.mit.edu> (document.domain=<http://web.mit.edu>).
JavaScript error: http://web.mit.edu/bzbarsky/www/foo.html, line 4: Permission denied for <http://web.mit.edu> (document.domain=<http://mit.edu>) to get property Window.document from <http://web.mit.edu> (document.domain=<http://web.mit.edu>).
JavaScript error: http://web.mit.edu/bzbarsky/www/test.html, line 5: Permission denied for <http://web.mit.edu> to get property Window.document from <http://www.yahoo.com>.
JavaScript error: http://web.mit.edu/bzbarsky/www/test.html, line 6: Permission denied for <http://web.mit.edu> (document.domain=<http://web.mit.edu>) to get property Window.document from <http://www.yahoo.com> (document.domain has not been set).
John, does that look ok?
Assignee: dveditz → bzbarsky
Status: NEW → ASSIGNED
Attachment #380167 -
Flags: superreview?(jst)
Attachment #380167 -
Flags: review?(l10n)
Attachment #380167 -
Flags: review?(jst)
| Reporter | ||
Comment 2•16 years ago
|
||
Yes that looks great. I will ask my colleague to assess it from his vantage as well.
Updated•16 years ago
|
Attachment #380167 -
Flags: review?(l10n) → review-
Comment 3•16 years ago
|
||
Comment on attachment 380167 [details] [diff] [review]
Proposed patch
r- for the localization notes. Most of our localizers don't use the source, but tools that display string by string, and those tools don't parse cleverly. I regret, but that's life.
A good localization note would look like
# LOCALIZATION NOTE (GetPropertyDeniedOriginsSubjectDomain):
# %1$S and %4$S are the origins of subject and object,
# %2$S is the js object and %3$S the property that cannot be accessed.
# %5$S is the document.domain set in the subject; don't translate "document.domain"
GetPropertyDeniedOriginsSubjectDomain = Permission denied for <%1$S> (document.domain=<%5$S>) to get property %2$S.%3$S from <%4$S> (document.domain has not been set).
even though I'm not happy with the "subject" and "object" yet. Mostly used that because I know what it is, but don't have the right words for it. We should just copy and paste such a note for each string and adapt.
| Assignee | ||
Comment 4•16 years ago
|
||
Attachment #380229 -
Flags: superreview?(jst)
Attachment #380229 -
Flags: review?(l10n)
Attachment #380229 -
Flags: review?(jst)
Updated•16 years ago
|
Attachment #380229 -
Flags: review?(l10n) → review+
Comment 5•16 years ago
|
||
Comment on attachment 380229 [details] [diff] [review]
Better localization notes
Looks good to me, thanks.
| Assignee | ||
Updated•16 years ago
|
Attachment #380167 -
Attachment is obsolete: true
Attachment #380167 -
Flags: superreview?(jst)
Attachment #380167 -
Flags: review?(jst)
Updated•16 years ago
|
Attachment #380229 -
Flags: superreview?(jst)
Attachment #380229 -
Flags: superreview+
Attachment #380229 -
Flags: review?(jst)
Attachment #380229 -
Flags: review+
| Assignee | ||
Comment 6•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
| Assignee | ||
Comment 7•16 years ago
|
||
And pushed a test crash fix as http://hg.mozilla.org/mozilla-central/rev/9d5e247b5052
| Assignee | ||
Comment 8•16 years ago
|
||
Backed both out:
http://hg.mozilla.org/mozilla-central/rev/069d94d0d7ef
http://hg.mozilla.org/mozilla-central/rev/78b2c479870c
to see whether this might have caused the WinXP Txul regression.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
| Assignee | ||
Comment 9•16 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
The l10n notes seem to be misleading.
+# LOCALIZATION NOTE (GetPropertyDeniedOrigins):
+# %1$S is the origin of the script which was denied access.
+# %2$S is the origin of the object access was denied to.
+# %3$S is the type of object it was.
+# %4$S is the property of that object that access was denied for.
+GetPropertyDeniedOrigins = Permission denied for <%1$S> to get property %2$S.%3$S from <%4$S>.
If the l10n note was true, the message would render somehow like this:
Permission denied for example.com to get property example.net.Window from document.
| Assignee | ||
Comment 11•16 years ago
|
||
Um.... yes. I have no idea how that happened....
| Assignee | ||
Comment 12•16 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/4d79fd0b8454 to fix that. Marek, thank you for catching the problem!
You need to log in
before you can comment on or make changes to this bug.
Description
•