Community
Participate
Working Groups
This is split out from bug 333972.
Created attachment 194222 [details] patch Patch contains java changes for all Browser implementations. The recompiled external.xpt will be in the next comment.
Created attachment 194223 [details] recompiled external.xpt
The current patch is good in cross-platform, but I am not sure if it is strong enough. At least, we should use a bigger token, e.g. two 32bits int. One 32bit int seem easy to be detected. The firefox 4 on my PC take less than 5 minutes to finish below code. -------------------------------------------- function loopToken(){ var i = 0; while ( i < 0x6FFFFFFF ){ var j = 0; while (j < 0xFF) { j++; i++; } document.getElementById("prog").innerHTML = i; var txt = document.getElementById("prog2").innerHTML; txt = ".." + txt; if (txt.length > 100) txt = ""; document.getElementById("prog2").innerHTML = txt; } } ------------------------------------------------------- After all, I would prefer to leverage the browser's security framework, e.g. the nsIScriptSecurityManager and nsIPrincipal for firefox/xulrunner.
The token came into the callJava is a pointer value. After called to convertToJava, it returned the correct double object. However I am getting a java core dump when the code processes args pointer on this line rc = variant.GetDataType (type); I am running on Windows 7, SWT browser with XULRunner 1.9.2.10
Grant, are you working for a bigger token and better token generator? Do you have an ETA?
Created attachment 194901 [details] secure iframe patch In the patch, remove the for loop code of "iframe[i].theJavaFunction = topFrame.theJavaFunction ", which was used for copying the function reference from top frame to child frame, and which may violate the cross-domain policy between frames.
This is still targetted for RC1. The token generation has been changed to the following, which now uses 53 bits and gives a better distribution of values: Random random = new Random (); long value = random.nextLong (); token = ((value & 0xFFE0000000000000L) >>> 11) ^ (value & 0x1FFFFFFFFFFFFFL) ^ hashCode ();
Grant, the new token generation sound great. Thank you! And, any thought on Yun's patch at comment#6?
(In reply to comment #7) Grant, seems the 53 bits token can not be passed into External:callJava() method, since the parameter of XPCOMObject's callback methods are all int types(e.g. "int /*long*/[] callbackArgs"). The token is truncated to 32bit when run into callJava method. May use 2 int type tokens is a good idea, what's your opinion?
Created attachment 195159 [details] patch with 53-bit token The attached patch now uses 53-bit tokens. It has not been reviewed yet, and does not currently contain the changes in comment 6. This patch and the comment 6 patch will be reviewed/considered tomorrow.
Created attachment 195160 [details] external.xpt for 53-bit tokens
(In reply to comment #10) The patch worked. But from security view, the token used still is 32bit, not 53bit, because the token used in window.external.callJava is an int type pointer, which is 32bit. And I verified the address pointer can be accessed by another browser instance with window.external.callJava js code( e.g. window.external.callJava(1,4072923143722503,['hi']);).
(In reply to comment #12) > The patch worked. But from security view, the token used still is 32bit, not > 53bit, because the token used in window.external.callJava is an int type > pointer, which is 32bit. And I verified the address pointer can be accessed by > another browser instance with window.external.callJava js code( e.g. > window.external.callJava(1,4072923143722503,['hi']);). The 32-bit tokenVariant argument to callJava is a pointer to an nsIVariant containing the token, not to the token itself. It contains the 53-bit token, which is accessed in convertToJava() with nsIVariant.GetAsDouble().
Created attachment 195272 [details] latest patch This patch has a couple of small changes, following the review with SSQ. It is released > 20110510. Regarding not hooking listeners on child frames, we still need to investigate this further because it is a behavior change. It can still be considered for 3.7RC1.
Marking as FIXED since calls to window.external.callJava() now require a 52-bit token, which was the focus of this report. The patch from comment 6 is applicable to original bug 333972, which is still open.
Grant, thanks for fixing this ! Would it be possible for you to attach a patch against 3.6.2 release?
I'll look into this, it could be a couple of days.
Created attachment 195546 [details] final 3.7-stream patch The previous 3.7-stream patch had an error in its WebKit on GTK implementation. This is now fixed in HEAD, and is contained in this updated patch.
Created attachment 195550 [details] patch for 3.6.2 Here's a patch that applies this report's changes against 3.6.2, including the small fix that was added in the previous comment. Remember that the external.xpt attached in comment 11 is also needed.
