recent trunk has restrictions on what local html can access in bug 402998 Comment #8 someone with sun.com email asked to "post a test" for local applet circumventing restrictions. it is like beating a death horse, but here it is: if the path of the locally saved applet is known at applet compile time, the applet can read any file. note that if the luser saves files in a single directory, a two stage attack may be successful with high probability. suppose the applet is saved in directory: /tmp/DumbUglyB1llMarriedDumbUglyB1tch it should be instantiated like this: <applet codebase="file:///" code="tmp.DumbUglyB1llMarriedDumbUglyB1tch.a1"> </applet> and the applet should contain: /* * This is the path to the applet filename: * */ package tmp.DumbUglyB1llMarriedDumbUglyB1tch; public class a1 extends Applet {

Agreed, this should not be possible and is a bug on the Java side. Appropriate people have been notified, and we will update this bug with progress.

when i was a coder in the old dos days, we were instructed to call our application bugs "uncontrollable process in the operating systems" whenever possible ;)

[sg:moderate] imo

georgi, is this still a problem with an up to date java plugin?

I just tested, and it still is a problem with the testcase as written. The reason being that the testcase sets a codebase of file:///, which means anything on the users filesystem is considered same origin. If I change the testcase to not set a codebase (and move the class file around so it's found) then I get a permission denied error. I guess we could consider something along the lines of forbidding setting of the codebase to a file:// uri at all, which would force the codebase to be the location of the containing page. Not great, but better than what we have now.

WIP patches #1 - Makes GetObjectBaseURI xpcom-able so we can use this as the canonical base URI, to avoid duplicating logic. It also adds a mimicry of java's empty-codebase quirk (previously we were security checking different URLs than java) #2 - Using the now-correct GetObjectBaseURI, check file-URLs for same-directory. #3 - In the code that caches attributes for communication with the plugin, force codebase to the processed result of GetObjectBaseURI, such that we ensure agreement upon codebase.

Simpler approach to this - since we don't know of any other java bugs/quirks with parsing the codebase, forcing the attribute to be the normalized path is probably overkill. These patches ensure GetObjectBaseURI() matches what the plugin will see by emulating java's quirk, and then runs a security check on same-directory for file URIs

Comment on attachment 597985 [details] [diff] [review] [2/2] Enforce same-directory for plugin codebase on file URIs It's unfortunately that we can't share some of this logic with nsPrincipal::CheckMayLoad(), but I don't really see an easy way to do that, given the differences. However, I'd like to see the guts of this file related stuff split out into its own function, that would simplify the return paths etc, you can do the fallback only once in the calling code, and it would make this code a bit easier to read. Other than that, this looks correct to me. r- to get this split into its own function.

So I discovered that "file:" also triggers bad behavior from java. This should switch the scheme to file but keep the path unchanged, but java also interprets this as "/" path on the new scheme. Because we don't particularly care about preserving this behavior, resurrect the canonicalize-codebase patch to ensure agreement (and effectively break anyone relying on "scheme:" codebases). This avoids adding even more code to mimic java (especially since we can't parse relative URIs easily with nsIURI) and protects against any other unexpected issues in its URI parsing.

Fix CnP error

Fix whitespace

fixup whitespace

Fixup whitespace, remove debugging LOG() statement

whitespace.

Survived a try build here https://tbpl.mozilla.org/?tree=Try&rev=498965274875

Comment on attachment 600483 [details] [diff] [review] [3/4] Run same-directory check on file URIs ... + nsCOMPtr<nsIURI> docURI; + nsCOMPtr<nsIURI> baseURI; + GetObjectBaseURI(aTypeHint, getter_AddRefs(baseURI)); + rv = thisContent->NodePrincipal()->GetURI(getter_AddRefs(docURI)); Given that docURI comes from the principal, I'd recommending doing s/doc(ument)?/origin/ in this whole patch, as the document uri can be different from the origin uri (especially when dealing with javascript: and data: URIs). Making that change would make this naming follow surrounding code better. + if (NS_FAILED(docURI->SchemeIs("file", &isfile)) || + (isfile && !IsFileCodebaseAllowable(baseURI.get(), docURI.get()))) { No need for those .get() calls. r=jst with those changes made.

Comment on attachment 600484 [details] [diff] [review] [4/4] Canonicalize the codebase pushed to java Looks good! r=jst

docURI renamed to originURI, and rolled into one since the latter wont apply cleanly without the former.

Comment on attachment 601053 [details] [diff] [review] Ensure we agree with java on applet codebase, and run security checks for file: codebase URIs. r=jst r=jst

This has caused a serious regression in Thunderbird - bug 731947 - Plugins are no longer blocked in email as per Thunderbird's content policy (or at least the application/x-test plugin isn't blocked).

So I've just confirmed that although the code is trying to set allowPlugins and allowJavascript to false on the docShell (this is via nsMsgContentPolicy::ShouldLoad), it is no longer happening on the message pane docShell.

Which is no longer happening? The call doesn't get to the docshell? Or the docshell setting is ignored? Is the issue that nsObjectLoadingContent::GetObjectBaseURI got changed for the case of not-applets so that the "instantiate without a URI" codepath is now not passing in a useful URI anymore and security checks that rely on that fail?

We should probably get a core bug followup filed blocking this one on the issue, and request tracking13 on it.

I'm looking into this

The problem is this call is failing: > nsContentUtils::NewURIWithDocumentCharset(aURI, codebase, > thisContent->OwnerDoc(), > baseURI); With codebase "" and base "mailbox:///home/johns/moz/tb-dbg/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/pluginPolicy?number=0" Causing this function to return NS_OK, but not return a URI. This could happen before, but only if the object had a codebase attribute, which the test doesn't cover. Attached patch returns the object's baseURI if codebase URI creation fails.

Why do you need to clone? Can you not swap like the old code used to?

@bz - The function is exposed to xpcom now, so the old assert(aURI = null) quirk is undesirable. (especially as this is a trivial function performance-wise)

> @bz - The function is exposed to xpcom now, so the old assert(aURI = null) quirk is > undesirable. If you want to, you can set *aURI to null before swapping to make the swap safe. I agree that just asserting that *aURI is null is no longer OK. > especially as this is a trivial function performance-wise Cloning some URIs can copy megabytes of data. Let's just not do it unless we really really have to. (On a separate note, why exactly is this function capitalized in the IDL? The convention it lowercase first letter in IDL....)

New patch - restores using uri swapping when possible, and fixes the capitalization in the idl

Comment on attachment 602461 [details] [diff] [review] Followup - Handle failed URI creation, fix typo Drop the unnecessary else after return, return the rv that succeeded instead of NS_OK, and r=me.

changes per bz, checkin-needed

Tracking for the ESR that is released alongside Firefox 13. We'll want to make sure that we fix bug 731947 at the same time.

The regression in Bug 736965 cannot be fully fixed without a bigger change to make our codebase detection in ObjectLoadingContent and cached parameters in PluginInstanceHost take <param> tags into consideration. Given that the code is very fragile and that we have almost no tests covering plugin loading and java specifically, this is rather risky regression-wise. There are also and there are other much bigger security issues in ObjectLoadingContent that require refactoring of the class. This is happening in Bug 738396, which will also include better tests for java loading. After talking with jst, I think the best course of action is to back this patch out of aurora, and address the regressions along with the further security issues in 738396.

[Approval Request Comment] Reason: The fix for this caused several regressions which are too risky to fix on aurora (see earlier comments). User impact if declined: java won't load in several cases Testing completed (on m-c, etc.): This is a backout to a known good state (with a moderate security bug w/o this fix). Risk to taking this patch (and alternatives if risky): We either take this, or we ship with java broken in significant ways, or we take a much riskier fix (which isn't quite done yet), i.e. not an option. String changes made by this patch: n/a

Comment on attachment 614666 [details] [diff] [review] aurora backout of the fix + followup in this bug. [Triage Comment] Approved for Aurora 13 given the regressions tracked in bug 736965 and bug 741758.

Backout landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/96a1b7f055c9

The original issue in comment 0 is fixed now. It is not entirely clear which changes the current bug state is tracking. Are we now tracking the status of the regressions caused by the initial checkin within this bug?

Comment on attachment 629267 [details] [diff] [review] Backout of aurora (14) for regressions Back out of 14 as well for the same reasons as above. Bug 745030 should be landing shortly as a proper fix for this as well as unfixed security issues in Bug 738396 - but wont be suitable for aurora. [Approval Request Comment] User impact if declined: Major plugin loading regressions, mainly with java. Testing completed (on m-c, etc.): Returning to known-good state Risk to taking this patch (and alternatives if risky): Re-introduces sg-moderate issue String or UUID changes made by this patch: Changes nsIObjectLoadingContent uuid (cannot revert to original UUID due to subsequent changes)

Comment on attachment 629267 [details] [diff] [review] Backout of aurora (14) for regressions [Triage Comment] Approving the backout. Please make sure to update the status flags once landed. Thanks!

Backed out from beta 14. https://hg.mozilla.org/releases/mozilla-beta/rev/4782adb17edf

I guess 15 is still "fixed", although we may get around to backing it out later if bug 745030 doesn't land soon.

Is FIrefox 14 a "wontfix", or do we expect to somehow get this checked in again? I'm assuming it's simply not in this release. Are the regressions likely to get fixed before Firefox 15 makes it to Beta? Maybe we should just back it out from there now if that's not likely.

Comment on attachment 634674 [details] [diff] [review] Backout of 15 Bug 745030 may not land this cycle and bug 738396 will need to land on top of it to fix further issues with java. Additionally, fully fixing the regressions here might require some of the changes in 738396. I talked with jst and he agrees the best course of action is to back this out from remaining branches, and re-land it merged with the 738396 changes once 745030 is landed (when it can be done without regressions) [Approval Request Comment] User impact if declined: Significant Java regressions Testing completed (on m-c, etc.): Returning to known good state Risk to taking this patch (and alternatives if risky): re-introduces sg-moderate issue String or UUID changes made by this patch: Changes nsIObjectLoadingContent UUID to match FF14

(In reply to John Schoenick [:johns] from comment #58) > String or UUID changes made by this patch: Changes nsIObjectLoadingContent > UUID to match FF14 Does this have any possibility of compatibility regressions? If we're not sure, we should loop somebody in from AMO (like jorgev).

(In reply to Alex Keybl [:akeybl] from comment #59) > Does this have any possibility of compatibility regressions? If we're not > sure, we should loop somebody in from AMO (like jorgev). I don't believe so - this removes a single call addons were not likely to be using (getObjectBaseURI) and thus changes the interface and its UUID to match what is on beta and aurora - addons that have compatibility there should be fine through trunk. Additionally, I believe we will get away with fixing this properly with 745030 + 738396 without changing the interface again, the changes in bug 745030 mean we don't actually need to export this function via the IDL

Comment on attachment 634674 [details] [diff] [review] Backout of 15 Thanks John and Johnny. Approving for Aurora 15 given r+ and the above risk assessment.

https://hg.mozilla.org/releases/mozilla-aurora/rev/419408a8db2f https://hg.mozilla.org/mozilla-central/rev/47f814827db6

Just to clarify, this was reopened because the last checkin here was a backout. There's a new version of this fix coming, but that builds on some significant changes that were needed in order to be able to fix this sanely.

Hang on, why was this marked fixed in 18? AFAIK this is still an issue, along with bug 738396. Bug 745030 and it's follow-ups make this possible to fix sanely, but any fix could not be landed on esr10, so nothing has been pushed yet.

Removing CVE number and advisory tags.

Bug 738396 fixes most of the codebase handling issues here, so all we need to fix this is the proper same-directory policy for file:/// URIs

Comment on attachment 601053 [details] [diff] [review] Ensure we agree with java on applet codebase, and run security checks for file: codebase URIs. r=jst This needs a new approach based on bug 745030 and bug 738396

Allows us to do same origin file checking while bypassing the no-directories rule

Based on the patches in bug 738396 -- add a same origin check to the codebase if the plugin is java and the codebase is a file URI

We presumably wont want to land the CheckMayLoad change on branches, this just duplicates that code into ObjectLoadingContent minus the directory check. This patch would be landed into the add-codebase-check patch on branches only.

Comment on attachment 733642 [details] [diff] [review] Add an allowFileDirectories argument to nsPrincipal::CheckMayLoad bz, would you be the right person to review this?

Comment on attachment 733644 [details] [diff] [review] Add check for java file codebase security Based on the fixes in bug 738396 - do a same-origin check on file URIs to enforce our strict file origin policy (but note that we don't same-origin check java otherwise :-/)

Comment on attachment 733645 [details] [diff] [review] [Fold into patch on branches] Move strict file origin checking blob into nsOBJLC If we take this on any branches we probably wont want to uplift the CheckMayLoad() change - this just duplicates some same-file-origin policy code into ObjectLoadingContent for branches only.

Comment on attachment 755520 [details] [diff] [review] Test Test that java plugins whose codebase violates the strict file origin policy don't spawn.

Why do we need that CheckMayLoad change, exactly?

(In reply to Boris Zbarsky (:bz) from comment #76) > Why do we need that CheckMayLoad change, exactly? We want to do a same origin check for java's codebase value when it is on the local filesystem, to enforce our strict file origin checks, but it needs to allow directory targets. We could move the strict file origin stuff out of CheckMayLoad to another utility function, but it constitutes the bulk of the function and didn't really seem any less messy. If you have a suggestion for a cleaner way I'm more than willing to rework it.

So the codebase can be a directory? I would rather have some sort of utility function called from both places, honestly, since CheckMayLoad() is really supposed to be about loading things and that's not what we're doing here. Unless we need logic from CheckMayLoad other than the file:// handling?

(In reply to Boris Zbarsky (:bz) from comment #78) > So the codebase can be a directory? > > I would rather have some sort of utility function called from both places, > honestly, since CheckMayLoad() is really supposed to be about loading things > and that's not what we're doing here. Unless we need logic from > CheckMayLoad other than the file:// handling? The problem is java assumes everything under codebase is fair game for it to load, but doesn't go through us to open channels, so checking that the page may load that base URI is as close as we can get. We normally skip the same-origin check, but we want to do the strict file same-origin check in the case of file:// URIs, which is what CheckMayLoad is enforcing. However, CheckMayLoad is now basically a wrapper around SecurityCompareURIs that also implements strict file origin. If we split it out, All that would remain of CheckMayLoad would be: > NS_IMETHODIMP > nsPrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, bool aAllowIfInheritsPrincipal) > { > if (aAllowIfInheritsPrincipal) { > // If the caller specified to allow loads of URIs that inherit > // our principal, allow the load if this URI inherits its principal > if (nsPrincipal::IsPrincipalInherited(aURI)) { > return NS_OK; > } > } > > if (!nsScriptSecurityManager::SecurityCompareURIs(mCodebase, aURI) && !CheckStrictFileOriginPolicy()) { > return NS_ERROR_DOM_BAD_URI; > } > > return NS_OK; > } So assuming that SecurityCompareURIs is going to succeed on two file:// URIs, the other checks are redundant. Though, if we did add some additional file restrictions to SecurityCompareURIs at some point, we would conceivably want them to apply here. Perhaps CheckMayLoad should just be merged with SecurityCompareURIs? Or the strict-file-origin policy moved to SecurityCompareURIs instead of CheckMayLoad?

Could we just disallow java on file: URIs completely (behind a pref, I guess)?

> So assuming that SecurityCompareURIs is going to succeed on two file:// URIs It's not. It's going to fail on two distinct file:// URIs if the file origin policy is strict. The CheckStrictFileOriginPolicy() bit actually allows the check to pass even though two different file:// URIs are normally considered different origins. > Perhaps CheckMayLoad should just be merged with SecurityCompareURIs? No, because SecurityCompareURIs is a stricter and symmetric check while CheckMayLoad is a looser and asymmetric check. > All that would remain of CheckMayLoad would be: That seems perfectly reasonable to me if we have a second consumer who in fact wants the modified CheckMayLoad file://-related behavior there.

Comment on attachment 733642 [details] [diff] [review] Add an allowFileDirectories argument to nsPrincipal::CheckMayLoad (In reply to Benjamin Smedberg [:bsmedberg] from comment #80) > Could we just disallow java on file: URIs completely (behind a pref, I > guess)? With the work on bug 738396 this is actually not too hard to fix, changes to CheckMayLoad aside. If java behaves and stays confined to its codebase its risk shouldn't be any higher than on a site, though it is java... (In reply to Boris Zbarsky (:bz) from comment #81) > > So assuming that SecurityCompareURIs is going to succeed on two file:// URIs > > It's not. It's going to fail on two distinct file:// URIs if the file > origin policy is strict. The CheckStrictFileOriginPolicy() bit actually > allows the check to pass even though two different file:// URIs are normally > considered different origins. > > > Perhaps CheckMayLoad should just be merged with SecurityCompareURIs? > > No, because SecurityCompareURIs is a stricter and symmetric check while > CheckMayLoad is a looser and asymmetric check. > > > All that would remain of CheckMayLoad would be: > > That seems perfectly reasonable to me if we have a second consumer who in > fact wants the modified CheckMayLoad file://-related behavior there. Okay, that sounds sane then. Do you have any preference as to where this should live?

Comment on attachment 733645 [details] [diff] [review] [Fold into patch on branches] Move strict file origin checking blob into nsOBJLC This patch is just duplicating the strict file origin code into nsObjectLoadingContent for branches only, it'll remain necessary independent of how we factor it on trunk

If we can put the file:// bits in nsNetUtil, I would vote there, but I haven't read them that carefully... Failing that, nsContentUtils?

Add these bits to nsNetUtil. URIIsLocalFile is wanted by nsObjectLoadingContent when it is doing the checks, so I promoted it as well.

Split out to avoid touching this on branches.

Use NS_CheckStrictFileOriginPolicy instead of CheckMayLoad, carrying over r+

Comment on attachment 758043 [details] [diff] [review] Add NS_URIIsLocalFile and NS_CheckStrictFileOriginPolicy >+ MOZ_ASSERT(util, "should have netutil"); Then why do we null-check it later? >+NS_CheckStrictFileOriginPolicy(nsIURI *aURI, This should probably be called NS_RelaxStrictFileOriginPolicy. And aURI should be aTargetURI, while aCodebaseURI is aSourceURI, I think, with appropriate renaming for fileURL, codebaseFileURL, codebaseIsDir, codebaseFile, codebaseParent. Otherwise it's not clear what's going on with this check; recall that this is an _asymmetric_ check, so the order of arguments is very important. Worth documenting exactly what this function is meant for and what the arguments mean. For example, your call to this function in the other patch got the order backwards... Which would have been easier to notice if the two changes had been in the same patch, by the way. That way blame would be easier to track too. Please do combine the patches like that. >+ (!aAllowDirectoryURIs && >+ (NS_FAILED(targetFile->IsDirectory(&targetIsDir)) || targetIsDir))) { That second line needs to be indented one more space. r=me with the above fixed.

Comment on attachment 758046 [details] [diff] [review] Use NS_CheckStrictOriginPolicy in CheckMayLoad This gets the check backwards. And this should be part of the patch that adds the new method anyway, I think.

I made enough changes to this that I'd appreciate a second look, this would be bad code to make a mistake in (like swapping argument orders) (In reply to Boris Zbarsky (:bz) from comment #88) > Comment on attachment 758043 [details] [diff] [review] > Add NS_URIIsLocalFile and NS_CheckStrictFileOriginPolicy > > >+ MOZ_ASSERT(util, "should have netutil"); > > Then why do we null-check it later? Removed > >+NS_CheckStrictFileOriginPolicy(nsIURI *aURI, > > This should probably be called NS_RelaxStrictFileOriginPolicy. > > And aURI should be aTargetURI, while aCodebaseURI is aSourceURI, I think, > with appropriate renaming for fileURL, codebaseFileURL, codebaseIsDir, > codebaseFile, codebaseParent. Otherwise it's not clear what's going on with > this check; recall that this is an _asymmetric_ check, so the order of > arguments is very important. Worth documenting exactly what this function > is meant for and what the arguments mean. I renamed these as such, and added an explanatory comment > For example, your call to this function in the other patch got the order > backwards... Which would have been easier to notice if the two changes had > been in the same patch, by the way. That way blame would be easier to track > too. Please do combine the patches like that. I originally split these such that CheckMayLoad could be unchanged should we land this on branches, but we can split it if deemed necessary at that point. > >+ (!aAllowDirectoryURIs && > >+ (NS_FAILED(targetFile->IsDirectory(&targetIsDir)) || targetIsDir))) { > > That second line needs to be indented one more space. > > r=me with the above fixed. (In reply to Boris Zbarsky (:bz) from comment #89) > Comment on attachment 758046 [details] [diff] [review] > Use NS_CheckStrictOriginPolicy in CheckMayLoad > > This gets the check backwards. And this should be part of the patch that > adds the new method anyway, I think. Indeed it does, I had swapped the argument order to the call at one point and neglected to update the callers. However, while trying to figure out why my test didn't catch this, I found that nsIFile::Contains() recursion argument is ignored now, so we actually don't implement file same origin policy as described at: https://developer.mozilla.org/en-US/docs/Same-origin_policy_for_file:_URIs -- I'll open a followup for this

Comment on attachment 758243 [details] [diff] [review] Split NS_CheckStrictFileOriginPolicy out of nsPrincipal::CheckMayLoad This looks great. r=me, and sorry for the lag....

Comment on attachment 758249 [details] [diff] [review] Add check for java file codebase security. r=bsmedberg [Security approval request comment] This is just an enhancement on top of bug 738396 to enforce our file origin policy to java's codebase for sites loaded from a file URI: https://developer.mozilla.org/en-US/docs/Same-origin_policy_for_file:_URIs It requires 738396, but does not introduce any significant risk on top of it. How easily could an exploit be constructed based on the patch? The only functional change is an explicit check for file strict origin policy for java codebase, so pretty easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? A new comment explains the check, which is pretty obvious regardless. Which older supported branches are affected by this flaw? All, but not relevant to b2g where java is not present. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should not be difficult to rebase on all branches where 738396 lands How likely is this patch to cause regressions; how much testing does it need? Fairly low, only adds one additional check to java's codebase, and only when loading a local file:/// URI. Might break some odd enterprise setups that involve running java on the local disk, but there is already a pref to disable strict file origin checking.

Re-tracking now that there's a ready patch

Comment on attachment 758249 [details] [diff] [review] Add check for java file codebase security. r=bsmedberg Sec-approval+ but since this is only a sec-moderate, it doesn't *require* sec-approval to go into trunk.

(In reply to Al Billings [:abillings] from comment #95) > Sec-approval+ but since this is only a sec-moderate, it doesn't *require* > sec-approval to go into trunk. Ack, good point.

Tracking the issue due to the risk called out in comment #93 and already marking it as wontfix as it looks like the dependent bug 738396 will be fixed on FX23.

Comment on attachment 758243 [details] [diff] [review] Split NS_CheckStrictFileOriginPolicy out of nsPrincipal::CheckMayLoad https://hg.mozilla.org/integration/mozilla-inbound/rev/9eafe94ea972

Comment on attachment 758249 [details] [diff] [review] Add check for java file codebase security. r=bsmedberg https://hg.mozilla.org/integration/mozilla-inbound/rev/b06bd5858999

https://hg.mozilla.org/mozilla-central/rev/9eafe94ea972 https://hg.mozilla.org/mozilla-central/rev/b06bd5858999

Are we intending to uplift these to 23/24?

Comment on attachment 758249 [details] [diff] [review] Add check for java file codebase security. r=bsmedberg [Approval Request Comment] Bug caused by (feature/regressing bug #): Not a FF bug, works around java having lax security checks with file:/// URIs User impact if declined: sec-low - saving a malicious site locally, with embedded java applets, and then running it, would allow read-only access to the full filesystem. Testing completed (on m-c, etc.): On m-c Risk to taking this patch (and alternatives if risky): Low, the bulk of the patch is moving functions around to be accessible by plugin code, and additional checks are only done for sites running on file:/// URIs. String or IDL/UUID changes made by this patch: None

Comment on attachment 758243 [details] [diff] [review] Split NS_CheckStrictFileOriginPolicy out of nsPrincipal::CheckMayLoad see comment 102

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #101) > Are we intending to uplift these to 23/24? beat me by 11 seconds :-P

https://hg.mozilla.org/releases/mozilla-aurora/rev/462bf273f287 https://hg.mozilla.org/releases/mozilla-aurora/rev/3967f5179aab This doesn't apply cleanly to beta without bug 738396. I'll land it there once bug 738396 gets ba+.

https://hg.mozilla.org/releases/mozilla-beta/rev/fc3d9e72357b https://hg.mozilla.org/releases/mozilla-beta/rev/363615b93e48

Comment on attachment 758249 [details] [diff] [review] Add check for java file codebase security. r=bsmedberg This was tracked for esr17, so nominating for a call on whether or not to uplift there -- See comment 102

Comment on attachment 758243 [details] [diff] [review] Split NS_CheckStrictFileOriginPolicy out of nsPrincipal::CheckMayLoad (see comment 107)

Since this is sec-low, let's be risk averse.

Add third case to test for issue found by bug 902375. This just adds a third applet tag to the existing logic, keeping r+

@dveditz - Can this bug and bug 738396 have their test cases landed now that this is on all branches? (Is there a specific policy for test cases and other follow ups as such?)

John, please land the tests. Generally, six to eight weeks after release of the fix to the public (or when the *following* release comes out) is the guideline.

Yes that's fine now, we stopped supporting ESR-17 (where this isn't fixed) in December.

Un-bitrotted

Comment on attachment 8379262 [details] [diff] [review] Test. r=bsmedberg Pushed test: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb56bd7c0feb Try: https://tbpl.mozilla.org/?tree=Try&rev=fd55a944e195

I backed this (and everything else from that push to inbound) out in http://hg.mozilla.org/integration/mozilla-inbound/rev/128cbf1edc40 due to various Java/plugin related failures they caused: Crashtest failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=35003251&tree=Mozilla-Inbound XPCShell failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=35003355&tree=Mozilla-Inbound

Attempt 2 at test: https://hg.mozilla.org/integration/mozilla-inbound/rev/277862f099a4 Extensive try run since this push burned the tree before: https://tbpl.mozilla.org/?tree=Try&rev=b656bdd77ce1

This was fixed in 25, the additional landing here only applies to the mochitest