Closed Bug 341313 Opened 14 years ago Closed 14 years ago

[FIX]CanExecuteScripts hard codes about:neterror as the only page that can execute script when scripts are disabled

Categories

(Core :: Security: CAPS, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: bugs, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

In nsScriptSecurityManager::CanExecuteScripts, about:neterror is hardcoded as the only local page that can execute scripts even when js is disabled. This list should instead be extendable, so that I can add about:feeds to it from the browser code without having to edit code in nsSSM.
Attached patch Proposed fixSplinter Review
I limited this to about: URIs for the time being.  If we want to have a protocol handler flag for this, we could do that, and add a third about: protocol handler for URIs which drop privs but want to allow script execution or something like that.  But I'm not sure we want to allow random other protocols to force script execution...
Assignee: dveditz → bzbarsky
Status: NEW → ASSIGNED
Attachment #226425 - Flags: superreview?(jst)
Attachment #226425 - Flags: review?(darin)
Priority: -- → P2
Summary: CanExecuteScripts hard codes about:neterror as the only page that can execute script when scripts are disabled → [FIX]CanExecuteScripts hard codes about:neterror as the only page that can execute script when scripts are disabled
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 226425 [details] [diff] [review]
Proposed fix

nit: When you called the method getURIFlags, I thought you intended to prefix all flags with URI_.  If you meant "get flags for this URI", then the method should probably have been called getFlagsForURI ;-)

r=darin
Attachment #226425 - Flags: review?(darin) → review+
I definitely meant the latter.  Do you want me to go ahead and rename the method?
Comment on attachment 226425 [details] [diff] [review]
Proposed fix

sr=jst
Attachment #226425 - Flags: superreview?(jst) → superreview+
> I definitely meant the latter.  Do you want me to go 
> ahead and rename the method?

Up to you.  It is probably not worth the time ;-)
Fixed.  I didn't change the name.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 226425 [details] [diff] [review]
Proposed fix

-    { "neterror", "chrome://global/content/netError.xhtml", PR_TRUE }
+    { "neterror", "chrome://global/content/netError.xhtml", PR_TRUE, PR_TRUE }

have you considered making the third field a "flags" field instead, so that the values are more self-documenting and that not all lines have to be changed for a new flag?
Hmm...  I suppose I could do that, yeah... file a bug and I'll deal in July?
QA Contact: caps
You need to log in before you can comment on or make changes to this bug.