User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051116 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051116 Firefox/1.6a1 The bug like bug 296231. If you set up application then you trust it almost extremely. There is a question what is "the instance replacing" as it is mentioned in a patch for bug 296231? Reproducible: Always

Comment on attachment 203490 [details] [diff] [review] patch Looks ok.

Checked into trunk.

Er... WHY are we doing a protocol check, not a real security check?

(In reply to comment #4) > Er... WHY are we doing a protocol check, not a real security check? > Don't you need a JSContext for real security checks to work?

No. As long as you have the nsIPrincipal involved (which you had better!), you can just call IsCapabilityEnabled() on it with whatever capability it is you have here. You'll have to pass null for the annotation, and at the moment that will make things equivalent to checking for the system principal, but that's what you want here, I assume. I certainly _hope_ that non-system chrome (eg themes) is not meant to get extra privileges here.

I guess we'll have to figure how to get a nsIPrincipal. cc: some more people. Perhaps we can use nsIScriptSecurityManager to get at it, not sure.

nsIDocument::GetPrincipal() should do what you want, I suspect. Also, imo, nsXFormsUtils::CheckSameOrigin should take an nsIPrincipal for the first arg and call checkSameOriginPrincipal, not checkSameOriginURI. In general any time you're working with URIs directly for security purposes you have a bug.

(In reply to comment #8) > nsIDocument::GetPrincipal() should do what you want, I suspect. > > Also, imo, nsXFormsUtils::CheckSameOrigin should take an nsIPrincipal for the > first arg and call checkSameOriginPrincipal, not checkSameOriginURI. > > In general any time you're working with URIs directly for security purposes you > have a bug. > Ouch. I was feeling a bit uneasy about this... but apparently it did not stop me from checking it in. We have some security issues then...

Yeah, I usually ask darin to review such changes, made a mistake by not doing this this time. Going to take a look.

(In reply to comment #8) > nsIDocument::GetPrincipal() should do what you want, I suspect. > > Also, imo, nsXFormsUtils::CheckSameOrigin should take an nsIPrincipal for the > first arg and call checkSameOriginPrincipal, not checkSameOriginURI. > > In general any time you're working with URIs directly for security purposes you > have a bug. > Shouldn't I use checkLoadURIWithPrincipal and pass in the principal of the document and the URI I want to load? Wouldn't that allow "true" chrome to access anything? Which would mean I can skip the IsCapabilityEnabled() check and just use nsXFormsUtils::CheckSameOrigin?

> Shouldn't I use checkLoadURIWithPrincipal and pass in the principal of the > document and the URI I want to load? Depends on the model you want. checkLoadURIWithPrincipal allows any http: page to load any other http: page, which is not what you want here, last I checked.

(In reply to comment #12) > > Shouldn't I use checkLoadURIWithPrincipal and pass in the principal of the > > document and the URI I want to load? > > Depends on the model you want. checkLoadURIWithPrincipal allows any http: page > to load any other http: page, which is not what you want here, last I checked. > I got it mostly working, what is the correct way to allow file:// to load anything?

Um.... There's no such way via the security manager that I know of. If we need one (and I suspect we do), file a bug?

This seems to work. I haven't tried chrome://, can you try with this Alexander? I am checking for the "UniversalBrowserRead" capability, which should work.

Comment on attachment 204376 [details] [diff] [review] use nsIPrincipal bz, do you have time to sr?

Comment on attachment 204376 [details] [diff] [review] use nsIPrincipal >Index: extensions/xforms/nsXFormsSubmissionElement.cpp >+nsXFormsSubmissionElement::CheckSameOrigin(nsIDocument >+ baseURI->SchemeIs("file", &allowSubmission); For what it's worth, this should be checking the rv -- any extension can add new protocols, and you can't trust them to not throw on SchemeIs. >Index: extensions/xforms/nsXFormsUtils.cpp >+ // check the security manager and do a same original check on the principal s/original/origin/ >+ rv = secMan->CheckSameOriginPrincipal(aBaseDocument->GetPrincipal(), Just pass in basePrincipal? sr=bzbarsky with those fixed.

Comment on attachment 204376 [details] [diff] [review] use nsIPrincipal allan, care to review the XForms changes?

Comment on attachment 204376 [details] [diff] [review] use nsIPrincipal (In reply to comment #18) > (From update of attachment 204376 [details] [diff] [review] [edit]) > allan, care to review the XForms changes? Yes, sorry that it took so long. r=me

checked in the nsIPrincipal patch.

checked into MOZILLA_1_8_BRANCH via bug 323691. Leaving open for now until it gets into 1.8.0