Last Comment Bug 738396 - Java applets may read arbitrary files on a user's system
: Java applets may read arbitrary files on a user's system
Status: VERIFIED FIXED
[sg:high][adv-main23+][adv-esr1708+]
: sec-high
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla25
Assigned To: John Schoenick [:johns]
: Matt Wobensmith [:mwobensmith][:matt:]
Mentors:
Depends on: 745030 783059 888834
Blocks: CVE-2013-1717
  Show dependency treegraph
 
Reported: 2012-03-22 12:12 PDT by John Schoenick [:johns]
Modified: 2014-11-19 20:03 PST (History)
19 users (show)
john: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
+
wontfix
+
wontfix
wontfix
wontfix
+
wontfix
+
wontfix
+
verified
+
verified
verified
wontfix
23+
verified
unaffected
unaffected


Attachments
Construct codebase URI for Java plugins as java would (11.21 KB, patch)
2013-03-29 15:22 PDT, John Schoenick [:johns]
benjamin: review+
jst: superreview+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr17+
abillings: sec‑approval+
john: checkin+
Details | Diff | Review
Only pass canonicalized codebase to Java (9.12 KB, patch)
2013-03-29 15:23 PDT, John Schoenick [:johns]
benjamin: review+
jst: superreview+
Details | Diff | Review
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches (5.90 KB, patch)
2013-03-29 15:23 PDT, John Schoenick [:johns]
no flags Details | Diff | Review
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches (5.96 KB, patch)
2013-04-01 11:22 PDT, John Schoenick [:johns]
benjamin: review+
Details | Diff | Review
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst (9.63 KB, patch)
2013-04-22 11:23 PDT, John Schoenick [:johns]
john: review+
john: superreview+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr17+
abillings: sec‑approval+
john: checkin+
Details | Diff | Review
Add nptest java plugin (13.72 KB, patch)
2013-05-29 11:16 PDT, John Schoenick [:johns]
benjamin: review+
Details | Diff | Review
Test (5.36 KB, patch)
2013-05-29 11:17 PDT, John Schoenick [:johns]
benjamin: review+
Details | Diff | Review
Test. r=bsmedberg (5.66 KB, patch)
2013-07-01 15:15 PDT, John Schoenick [:johns]
john: review+
Details | Diff | Review
Test case demonstrating this issue (1.95 KB, application/zip)
2013-07-09 11:26 PDT, John Schoenick [:johns]
no flags Details
Test. r=bsmedberg (7.06 KB, patch)
2014-02-20 14:33 PST, John Schoenick [:johns]
john: review+
john: checkin+
Details | Diff | Review

Description John Schoenick [:johns] 2012-03-22 12:12:16 PDT
Investigating 736965 which is fallout from a related security issue 406541, I discovered the fixes in 406541 are woefully inadequate. There are numerous ways to invoke java plugins which bypass various security checks. I found at least three different issues in the loading code. All of the below issues apply to 3.6+ and probably older.

The most severe of these is <param> elements can override the <object> tag's codebase as seen by java. That means this will load from a non-file: context

>  <applet width=500 height=500 codebase="." code="home.user.some.folder.object.class">
>    <param name="codebase" value="file:///">
>  </applet>

The security check will assume codebase "." is valid, but java will use the file:/// codebase, allowing read access to the full filesystem. There is no prompt when this applet loads.

For this to be successful, you must know the location of your applet on the user's machine - either through guessing a cache path, tricking the user into downloading it before hand, or possibly using something like flash to create a file with a more deterministic name.

Other issues in this code:
 - <object> tags default codebase to the current directory, rather than domain-root, unlike <applet> tags. We need to emulate this quirk to avoid breakage like 736965
 - <param> tags appear to be passed as raw html to java, leading to potential disagreements between parsers on malformed HTML.
 - Some code checks for application/x-java-vm to trigger java quirks, but it is possible with <embed> to create a an applet with e.g. application/x-java-applet. The code that forces the MIME to x-java-vm only applies to <object> tags.
- nodePrinicpal()->GetURI() is returning NULL on object tags in one situation with <object> tags, causing security checks to be skipped.
- (probably non-issue) according to oracle docs, <applet> should follow HTML4 specs, which state "Values for [codebase] may only refer to subdirectories of the directory containing the current document". This is never enforced (except on file:/// URIs with the patch in 406541), and contradict's java's behavior of defaulting codebase to "/" for <applet>
Comment 1 Daniel Veditz [:dveditz] 2012-09-13 13:36:43 PDT
Probably not reasonable to fix and uplift to Beta (16) at this point, but hopefully we can figure out a fix appropriate for 17.
Comment 2 Lukas Blakk [:lsblakk] use ?needinfo 2012-09-26 14:47:34 PDT
John, we're a little over a week away from moving 17 to the Beta audience - any work done on this yet and if not can you give an idea of when you'll be working on it?
Comment 3 John Schoenick [:johns] 2012-09-26 15:59:13 PDT
I've been waiting on landing more followups to this code until bug 783059 lands, as it's especially regression-prone (see original landing of bug 406541). I will try to have something ready before the uplift.
Comment 4 Alex Keybl [:akeybl] 2012-10-23 16:01:54 PDT
(In reply to John Schoenick [:johns] from comment #3)
> I've been waiting on landing more followups to this code until bug 783059
> lands, as it's especially regression-prone (see original landing of bug
> 406541). I will try to have something ready before the uplift.

Any updates here John? Is there something low risk we can do to resolve this issue in the FF17 timeframe?
Comment 5 Al Billings [:abillings] 2012-11-01 10:08:46 PDT
I've emailed John about this.
Comment 6 Lukas Blakk [:lsblakk] use ?needinfo 2012-11-07 17:49:07 PST
Now that beta 5 has gone to build we're past taking speculative fixes. Wontfixing for 17.
Comment 7 Al Billings [:abillings] 2013-01-03 13:21:37 PST
Is this still a current sec-high issue?
Comment 8 Andrew McCreight [:mccr8] 2013-03-21 13:35:25 PDT
John, what is the current status of this?  This is the oldest open DOM sec-high or sec-critical goal. :P
Comment 9 John Schoenick [:johns] 2013-03-29 15:22:53 PDT
Created attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would
Comment 10 John Schoenick [:johns] 2013-03-29 15:23:04 PDT
Created attachment 731338 [details] [diff] [review]
Only pass canonicalized codebase to Java
Comment 11 John Schoenick [:johns] 2013-03-29 15:23:15 PDT
Created attachment 731339 [details] [diff] [review]
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches
Comment 12 John Schoenick [:johns] 2013-03-29 15:35:13 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

So this got hairy fast, but I think should be decently-safe in that most of the changes reside within an |if (isJava)| block, so worst-case scenario is we break java...

The problem is multi-fold:
- Java uses "codebase" as a special value for where it is allowed to run, e.g. |file:///|, and assumes the browser will security-check it (despite not parsing it the way any browser would...)
- Java honors the last "codebase" value it sees, either on in tag attributes *or* in <param> children
- nsPluginInstanceOwner has a good deal of quirks relating to how it passes parameters and attributes to plugins
- nsObjectLoadingContent just looks at the attribute, not params, and without any of these quirks.
- Java doesn't parse URIs like the rest of the universe. E.g. no-codebase means ".", codebase="" means "/", and codebase="file:" means "file:///"

I opened Bug 853995 to move building the parameters we pass to the plugin to content (instead of having nsPluginInstanceOwner poke at the DOM), but that is non-trivial and wouldn't be suitable for branches. Instead, this patch basically copy-pastes a chunk of nsPluginInstanceOwner logic into nsObjectLoadingContent so they can arrive at the same codebase, with a patch for bug 853995 in progress to remove the code duplication from trunk.

Another issue is that we now parse the URI differently if we end up with Java, but can look at the URI's file extension in some cases and change plugin type. For example |<embed src="file.class">| will build file.class as a normal URI, before seeing .class and switching to java. I sort-of-elegantly fixed this by having UpdateObjectParameters re-call itself with a aJavaURI parameter at this point to redo the URI building steps with |isJava = true|.

Finally, now that we made a reasonable attempt to build the same mBaseURI that java would use, I added a java-only CheckLoadURIWithPrincipal security check against mBaseURI.

So risk-wise this patch looks scary, but is largely known-good code from nsPluginInstanceOwner, and fallout would likely only affect java plugins.
Comment 13 John Schoenick [:johns] 2013-03-29 15:37:16 PDT
Comment on attachment 731338 [details] [diff] [review]
Only pass canonicalized codebase to Java

This causes nsPluginInstanceOwner to get the parsed-and-canonicalized codebase from nsObjectLoadingContent, and pass only that as a "codebase" value in the case of java plugins.

This guarantees that other, undiscovered quirks in how java determines its codebase would simply result in us overriding the codebase to our interpretation, instead of security-checking a different codebase than java decides to use.
Comment 14 John Schoenick [:johns] 2013-03-29 15:38:11 PDT
Comment on attachment 731339 [details] [diff] [review]
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches

This would be folded against the canonicalize-codebase patch for branches only -- it moves the GetBaseURI() function to nsObjectLoadingContent.h and uses a nasty static-cast to get at it to avoid changing the nsIObjectLoadingContent IID on release branches.
Comment 15 John Schoenick [:johns] 2013-03-29 15:40:17 PDT
This was previously tracked (and not fixed...), re-noming now that we have patches.
Comment 16 Andrew McCreight [:mccr8] 2013-03-29 16:21:47 PDT
Too late for 20 at this point.
Comment 17 John Schoenick [:johns] 2013-04-01 11:22:14 PDT
Created attachment 731956 [details] [diff] [review]
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches

Inadvertently attached old version of this patch that doesn't compile...
Comment 18 Alex Keybl [:akeybl] 2013-04-01 14:21:10 PDT
We'll track for the ESR once this is resolved. Since this is a sec-high and appears to carry some plugin risk, we'll want to land ASAP on branches and follow up with QA regression testing. Any guidance there would be very helpful.

If this isn't ready to land in time for our second or third beta, we should strongly consider sec-approving next cycle instead.
Comment 19 Matt Wobensmith [:mwobensmith][:matt:] 2013-04-05 14:03:01 PDT
QA is ready to test when this gets landed. Basic concept is well-understood. Let me know if there are any other similar things to look for (param element vs object params, other types of paths, etc.).
Comment 20 Benjamin Smedberg [:bsmedberg] 2013-04-08 08:28:32 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

I'd like a second review on these patches; tagging jst though he might redirect.
Comment 21 Benjamin Smedberg [:bsmedberg] 2013-04-08 08:30:08 PDT
Comment on attachment 731338 [details] [diff] [review]
Only pass canonicalized codebase to Java

How do we test this?
Comment 22 Benjamin Smedberg [:bsmedberg] 2013-04-08 08:36:46 PDT
Comment on attachment 731956 [details] [diff] [review]
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches

Kinda hacky, but I think we're safe here since web content can't spoof nsIObjectLoadingContent.
Comment 23 Johnny Stenback (:jst, jst@mozilla.com) 2013-04-08 18:18:48 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

- In nsObjectLoadingContent::UpdateObjectParameters():

+    mydomElement->GetElementsByTagNameNS(xhtml_ns, NS_LITERAL_STRING("param"),
+                                         getter_AddRefs(allParams));
+    if (allParams) {
+      uint32_t numAllParams;
+      allParams->GetLength(&numAllParams);
+      for (uint32_t i = 0; i < numAllParams; i++) {
+        nsCOMPtr<nsIDOMNode> pnode;
+        allParams->Item(i, getter_AddRefs(pnode));
+        nsCOMPtr<nsIDOMElement> domelement = do_QueryInterface(pnode);
+        if (domelement) {
+          nsAutoString name;
+          domelement->GetAttribute(NS_LITERAL_STRING("name"), name);
+          name.Trim(" \n\r\t\b", true, true, false);
+          if (name.EqualsIgnoreCase("codebase")) {
+            // Find the first plugin element parent
+            nsCOMPtr<nsIDOMNode> parent;
+            nsCOMPtr<nsIDOMHTMLObjectElement> domobject;
+            nsCOMPtr<nsIDOMHTMLAppletElement> domapplet;
+            pnode->GetParentNode(getter_AddRefs(parent));
+            while (!(domobject || domapplet) && parent) {
+              domobject = do_QueryInterface(parent);
+              domapplet = do_QueryInterface(parent);
+              nsCOMPtr<nsIDOMNode> temp;
+              parent->GetParentNode(getter_AddRefs(temp));
+              parent = temp;
+            }

You could do all this with far less reference counting and QI calls if you used NS_GetContentList() and the internal interfaces, but I'm not convinced it's worth rewriting this for that alone.

+    codebaseStr.AssignLiteral("/");
+    // XXX(johns): This doesn't cover the case of "https:" which java would
+    //             interpret as "https:///" but weinterpret as this document's

Insert the missing space in weinterpret.

sr=jst
Comment 24 Johnny Stenback (:jst, jst@mozilla.com) 2013-04-08 19:46:33 PDT
Comment on attachment 731338 [details] [diff] [review]
Only pass canonicalized codebase to Java

- In nsPluginInstanceOwner::EnsureCachedAttrParamArrays():

+  // (Bug XXXXXX FIXME) java has quirks in its codebase parsing, pass the
+  // absolute codebase URI as content sees it.

Replace XXXXXX with a bug number there?

+    mCachedAttrParamNames [nextAttrParamIndex] = ToNewUTF8String(NS_LITERAL_STRING("codebase"));
+    mCachedAttrParamValues[nextAttrParamIndex] = ToNewUTF8String(NS_ConvertUTF8toUTF16(codebaseStr));

Wow, that's a pretty wonky string copy pattern we've got going here. Maybe file a followup bug to replace the above patterns with strdup() in this whole file?

sr=jst
Comment 25 John Schoenick [:johns] 2013-04-09 13:27:42 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #23)
> You could do all this with far less reference counting and QI calls if you
> used NS_GetContentList() and the internal interfaces, but I'm not convinced
> it's worth rewriting this for that alone.

(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #24)
> Wow, that's a pretty wonky string copy pattern we've got going here. Maybe
> file a followup bug to replace the above patterns with strdup() in this
> whole file?

This is all just copy-pasted nsPluginInstanceOwner code -- I opened bug 853995 to clean this up and de-duplicate it.
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2013-04-09 15:17:16 PDT
(In reply to John Schoenick [:johns] from comment #25)
> (In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #23)
> > You could do all this with far less reference counting and QI calls if you
> > used NS_GetContentList() and the internal interfaces, but I'm not convinced
> > it's worth rewriting this for that alone.
> 
> (In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #24)
> > Wow, that's a pretty wonky string copy pattern we've got going here. Maybe
> > file a followup bug to replace the above patterns with strdup() in this
> > whole file?
> 
> This is all just copy-pasted nsPluginInstanceOwner code -- I opened bug
> 853995 to clean this up and de-duplicate it.

Yup, didn't mean to suggest that this was your doing, it's all over this file from way long ago (and you did the right thing in following the existing coding patterns here)! Thanks for filing the followup!
Comment 27 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-04-18 12:28:01 PDT
Dropping qawanted but leaving verifyme. Matt is assigned to verify this once it lands.
Comment 28 John Schoenick [:johns] 2013-04-22 11:23:46 PDT
Created attachment 740385 [details] [diff] [review]
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst

Fix possible off-by-one, carrying over r
Comment 29 bhavana bajaj [:bajaj] 2013-04-29 23:14:17 PDT
Given https://bugzilla.mozilla.org/show_bug.cgi?id=738396#c18 and considering we are just two beta's away for Fx21 wontfixng this for fx21 . This will be a better fit for next cycle .
Comment 30 Matt Wobensmith [:mwobensmith][:matt:] 2013-05-21 15:47:35 PDT
Hello all - any movement here?
Comment 31 John Schoenick [:johns] 2013-05-29 11:16:28 PDT
Created attachment 755508 [details] [diff] [review]
Add nptest java plugin
Comment 32 John Schoenick [:johns] 2013-05-29 11:17:02 PDT
Created attachment 755510 [details] [diff] [review]
Test
Comment 33 John Schoenick [:johns] 2013-05-29 11:18:30 PDT
Comment on attachment 755508 [details] [diff] [review]
Add nptest java plugin

Adds another target for nptest that claims x-java-test, and adds a getJavaCodebase function, so we can test that we're passing the intended codebase to java tags.
Comment 34 John Schoenick [:johns] 2013-05-29 11:19:12 PDT
Comment on attachment 755510 [details] [diff] [review]
Test

Uses the new java test plugin target to ensure we see the expected codebase in various situations.
Comment 35 John Schoenick [:johns] 2013-05-29 11:25:11 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Pretty clearly reveals we are mis-handling java codebase, if the linked bug is also a restricted bug, this would quickly give someone the information they need to figure out the issue.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The code is pretty clear that we didn't audit java's codebase previously.

Which older supported branches are affected by this flaw?
All current branches. Irrelevant to b2g as we don't support plugins there.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Yes. Some of the riskier changes are replaced with simplier workarounds for branches only (the foldme patches)

How likely is this patch to cause regressions; how much testing does it need?
Low-to-moderate chance of introducing regressions, which would likely only affect certain configurations of java embedding.
Comment 36 John Schoenick [:johns] 2013-05-29 11:25:39 PDT
Comment on attachment 740385 [details] [diff] [review]
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst

[Security approval request comment]
(See above)
Comment 37 John Schoenick [:johns] 2013-05-29 11:27:12 PDT
Comment on attachment 731956 [details] [diff] [review]
[Branches only, foldme] Avoid changing nsIObjectLoadingContent IID on branches

[Security approval request comment]
See above. This just avoids changing the nsIObjectLoadingContent IID on branches.
Comment 38 Alex Keybl [:akeybl] 2013-05-31 13:42:59 PDT
(In reply to John Schoenick [:johns] from comment #35)
> Do comments in the patch, the check-in comment, or tests included in the
> patch paint a bulls-eye on the security problem?
> The code is pretty clear that we didn't audit java's codebase previously.
>
> How likely is this patch to cause regressions; how much testing does it need?
> Low-to-moderate chance of introducing regressions, which would likely only
> affect certain configurations of java embedding.

Give this, I think we should be cautious and fix for the first time in 23/24/25 and the corresponding ESR, to get a full 6 weeks of testing before release.
Comment 39 Al Billings [:abillings] 2013-05-31 15:52:54 PDT
The problem is

> [Security approval request comment]
> How easily could an exploit be constructed based on the patch?
> Pretty clearly reveals we are mis-handling java codebase, if the linked bug
> is also a restricted bug, this would quickly give someone the information
> they need to figure out the issue.
> 
> Do comments in the patch, the check-in comment, or tests included in the
> patch paint a bulls-eye on the security problem?
> The code is pretty clear that we didn't audit java's codebase previously.

This means that once we check in, we're going to be pretty exposed for the whole six week cycle.
Comment 40 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-05-31 16:04:19 PDT
Given this bug's potential impact (this is less severe than remote code execution, and isn't necessarily easy to exploit), I think favoring extra bake time over reduced trunk exposure seems reasonable.
Comment 41 John Schoenick [:johns] 2013-05-31 16:13:55 PDT
There is also bug 406541, which is a much less severe bug that depends on the changes here to fix. We could open a public version of that bug, then land these changes folded with 406541 as a "cleanup java codebase handling" patch. Without a locked bug to bring attention to it and the valid-premise for cleaning up this code, it's less obvious that there's a more severe issue to be found.
Comment 42 Al Billings [:abillings] 2013-05-31 17:19:54 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

Approving for trunk AFTER trunk becomes Firefox 25 on 6/24 or 25.
Comment 43 Al Billings [:abillings] 2013-05-31 17:20:03 PDT
Comment on attachment 740385 [details] [diff] [review]
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst

Approving for trunk AFTER trunk becomes Firefox 25 on 6/24 or 25.
Comment 44 Benjamin Smedberg [:bsmedberg] 2013-06-03 09:26:31 PDT
Comment on attachment 755510 [details] [diff] [review]
Test

The printf in nptest.cpp seems like a testing leftover? r=me with that gone
Comment 45 John Schoenick [:johns] 2013-06-27 13:59:24 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

https://hg.mozilla.org/integration/mozilla-inbound/rev/de553526d1f8
Comment 46 John Schoenick [:johns] 2013-06-27 13:59:45 PDT
Comment on attachment 740385 [details] [diff] [review]
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d38d2a6e4c1
Comment 48 John Schoenick [:johns] 2013-07-01 15:15:11 PDT
Created attachment 769872 [details] [diff] [review]
Test. r=bsmedberg

Folds in a few more cases for the regression in bug 888834, carrying over r+
Comment 49 John Schoenick [:johns] 2013-07-03 13:44:53 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
Technically a java bug: It assumes the browser will security-check its plugin-specific parameters, but we do not.

User impact if declined:
sec-high. Potential for untrusted java applets to get read-only access to the user's filesystem if used in conjunction with some method to download a file to a guessable path.

Testing completed (on m-c, etc.):
Has test case, on m-c. One minor regression found in bug 888834, which is now fixed and would be folded into the uplift patch.

Risk to taking this patch (and alternatives if risky):
In relation to java, there is a moderate regression risk. Java does not construct its "codebase" attribute in a very sane fashion, mis-constructing URLs and assuming |codebase=""| means |codebase="/"| among other things. Because we now force agreement on the codebase parameter, any quirks in java we did not emulate properly would break some java applets (e.g. bug 888834). These would be trivial fixes, if found.

Outside of embedded java, the risk is low, as all new code only applies to Java.

String or IDL/UUID changes made by this patch:
None
Comment 50 John Schoenick [:johns] 2013-07-03 13:45:40 PDT
Comment on attachment 740385 [details] [diff] [review]
Only pass canonicalized codebase to Java. r=bsmedberg sr=jst

(see comment 49)
Comment 51 Lukas Blakk [:lsblakk] use ?needinfo 2013-07-04 14:45:09 PDT
Comment on attachment 731337 [details] [diff] [review]
Construct codebase URI for Java plugins as java would

after 6/25 so let's uplift.
Comment 52 Ryan VanderMeulen [:RyanVM] 2013-07-08 06:57:24 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/819cefc60ade
https://hg.mozilla.org/releases/mozilla-aurora/rev/20a1cd5be4fb

There are some conflicts in nsObjectLoadingContent.cpp on esr17 that will need resolving. Also, this requires binary approval to land on beta due to the changes to nsIObjectLoadingContent.idl (comment 49 said no IDL changes?!?!).
Comment 53 John Schoenick [:johns] 2013-07-08 13:57:50 PDT
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #52)
> Also, this requires binary approval to land on beta due to
> the changes to nsIObjectLoadingContent.idl (comment 49 said no IDL
> changes?!?!).

This is why the |Avoid changing IID on branches| patch exists. I should've clarified in the whiteboard, apologies.

https://hg.mozilla.org/releases/mozilla-beta/rev/24df0a207416
https://hg.mozilla.org/releases/mozilla-beta/rev/10544c91e1d6
Comment 55 John Schoenick [:johns] 2013-07-09 11:26:10 PDT
Created attachment 772768 [details]
Test case demonstrating this issue
Comment 56 Matt Wobensmith [:mwobensmith][:matt:] 2013-07-10 16:52:46 PDT
QA has completed an extensive pass on this bug and signs off.

Confirmed original path loading exploit on OpenJDK6, Linux, FF22.
Verified fixed in FF25 m-c, 2013-07-08.
Verified fixed in FF24, 2013-07-10.
Verified fixed in FF23, 2013-07-10.
Verified fixed in FF17esr, 2013-07-10.

Verified that:
- Bad paths prevent JRE from loading applet at all, or
- Bad paths cause applet not to display, with error from JRE (security error or class not found).
- Normal use cases for applet loading - via object, embed and applet tags - work correctly.
- Other test cases for applet loading - nested tags, redirects, site vs root relative URLs, edge cases - work correctly.

JDKs used:
OpenJDK6u45/Ubuntu 13.0.4
OpenJDK7u21/Ubuntu 13.0.4
Oracle JDK 7u21/Ubuntu 13.0.4
Oracle JDK 7u25/Ubuntu 13.0.4
Oracle JDK 7u21/Mac OSX 10.8
Oracle JDK 7u25/Mac OSX 10.8
Comment 57 Kevin Brosnan [:kbrosnan] 2014-01-27 10:34:44 PST
*** Bug 964055 has been marked as a duplicate of this bug. ***
Comment 58 Andrew McCreight [:mccr8] 2014-01-27 10:42:01 PST
The blog post linked in bug 964055 doesn't talk about Java plugins at all.
Comment 59 John Schoenick [:johns] 2014-02-20 14:33:12 PST
Created attachment 8379263 [details] [diff] [review]
Test. r=bsmedberg

Un-bitrotted, rebased on java test plugin which was split out and added in bug 971279
Comment 61 Wes Kocher (:KWierso) 2014-02-20 16:23:08 PST
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
Comment 62 John Schoenick [:johns] 2014-02-25 13:34:29 PST
(In reply to Wes Kocher (:KWierso) from comment #61)
> 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 to push test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19a4abd04dce

Extensive try run since this push burned the tree before:
https://tbpl.mozilla.org/?tree=Try&rev=b656bdd77ce1
Comment 63 Wes Kocher (:KWierso) 2014-02-25 20:15:42 PST
https://hg.mozilla.org/mozilla-central/rev/19a4abd04dce
Comment 64 John Schoenick [:johns] 2014-02-26 12:00:50 PST
This was fixed in 25, the additional landing here only applies to the mochitest

Note You need to log in before you can comment on or make changes to this bug.