Closed Bug 495176 Opened 11 years ago Closed 11 years ago

[FIX]Permission denied message could include document.domain was set

Categories

(Core :: Security: CAPS, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: johnjbarton, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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: 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
Attached patch Proposed patch (obsolete) — Splinter Review
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)
Yes that looks great. I will ask my colleague to assess it from his vantage as well.
Attachment #380167 - Flags: review?(l10n) → review-
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.
Attachment #380229 - Flags: superreview?(jst)
Attachment #380229 - Flags: review?(l10n)
Attachment #380229 - Flags: review?(jst)
Attachment #380229 - Flags: review?(l10n) → review+
Comment on attachment 380229 [details] [diff] [review]
Better localization notes

Looks good to me, thanks.
Attachment #380167 - Attachment is obsolete: true
Attachment #380167 - Flags: superreview?(jst)
Attachment #380167 - Flags: review?(jst)
Attachment #380229 - Flags: superreview?(jst)
Attachment #380229 - Flags: superreview+
Attachment #380229 - Flags: review?(jst)
Attachment #380229 - Flags: review+
Pushed http://hg.mozilla.org/mozilla-central/rev/b55e7e3c0bfb
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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 → ---
Relanded as http://hg.mozilla.org/mozilla-central/rev/ad3c5bd01693
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
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.
Um.... yes.  I have no idea how that happened....
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.