User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.7.12) Gecko/20050915 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he; rv:1.8b4) Gecko/20050913 Firefox/1.4 when entering the account management in Bank Hapoalim web site, firefox 1.5beta1 indicates that the page is only partially enchrypted. the lock at the status bar and location bar is closed, with a red line across it. SeaMonkey 1.0alpha (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050910 SeaMonkey/1.0a) indicates the same problem. on the other side, firefox 1.0.7 and mozilla suite 1.7.12 do not, and show the page to be totally enchrypted. Reproducible: Always Steps to Reproduce: 1. open an account at Bank Hapoalim 2. sign the papers for internet access to the account 3. go to the above url and enter your account Actual Results: browser reports that some elements in the page are not enchrypted Expected Results: since i didn't find the string "http:" in the three main frames that make the page, i assume i should expect to see a full enchryption indication. here is the frameset they use: <FRAMESET COLS="8, *, 8" border="0" onUnload="doOnUnLoad()" onBeforeUnload="doOnUnLoad()" framespacing="0" frameborder="0" id="FRAMESET_TIME_OUT"> <FRAME noresize="true" marginwidth="0" name="SPACE_LEFT" SRC="javascript:top.nullPage(1);" scrolling="no" marginheight="0"></FRAME><FRAMESET ROWS="94,*" border="0" framespacing="0" frameborder="0"> <FRAME noresize="true" marginwidth="0" name="UPPER" SRC="javascript:top.nullPage(1);" scrolling="no" marginheight="0"></FRAME><FRAMESET COLS="*, 170, 3" border="0" framespacing="0" frameborder="0" id="FRAMESET_BODY"> <FRAMESET ROWS="0,*" border="0" onLoad="top.initMenus();" framespacing="0" frameborder="0"> <FRAME marginwidth="0" name="STAM" SRC="/cgi-bin/poalwwwc?reqName=stamFrame&u=p" scrolling="no" marginheight="0"></FRAME><FRAME marginwidth="0" name="BODY" SRC="javascript:top.nullPage(1);" scrolling="auto" marginheight="0"></FRAME></FRAMESET> <FRAME marginwidth="0" name="LOWER" SRC="javascript:top.nullPage(1);" scrolling="auto" marginheight="0"></FRAME><FRAME noresize="true" marginwidth="0" name="RED_LINE" SRC="javascript:top.nullPage(1);" scrolling="no" marginheight="0"></FRAME></FRAMESET></FRAMESET><FRAME noresize="true" marginwidth="0" name="SPACE_RIGHT" SRC="javascript:top.nullPage(1);" scrolling="auto" marginheight="0"></FRAME> <NOFRAMES> This site requires at least Netscape 2.0, with JavaScript Enabled. </NOFRAMES></FRAMESET> it comes after many lines of javascript code. i can't know how many through, because the status bar was striped from firefox' source viewer.

Steps 1 and 2 are rather large hurdles. Could you capture an http log and attach it to the bug? we may or may not learn something that way, but might show if the site itself is to blame (or eliminate that possibility). See http://www.mozilla.org/projects/netlib/http/http-debugging.html I only want the http headers, so "set NSPR_LOG_MODULES=nsHttp:5" leaving out the socket and host resolver bits. It would be most useful if you did this for 1.0.7 (working) and then the latest buggy Firefox or Seamonkey. Note that the log file gets overwritten each time you launch so either move it between runs, or set it to a different name when you are about to run the other version. It would help keep the log file size down if you launch and go right to the bank and then quit. You'll probably want to edit the files to remove any Cookie lines. If the site has decent security the cookies will only be useless tokens tied to your IP address and that expire quickly, but why take the chance? Ditto for any part of a URL after the first '?' -- I don't need those either and they might reveal account information If you want you can mail the logs directly to me, and I'll attach them to the bug only if they show something worth following up.

dveditz, i sent you a zip with the logs by email, from my other address.

Thanks for the logs, these look clean so I'm attaching them. Of course I can't tell for sure how the HTML is structured, but in both Firefox logs we load the non-ssl http://students.bankhapoalim.co.il/cgi-bin/inetcgi/student/exitGame.jsp and images/scripts/css referenced from that file. Is this loaded in a frame, and therefore FF1.5 is fixing a 1.0.x bug? Or is that just something that was loaded while you're exiting and the problem is elsewhere. While you're on the mixed page could you look in Page Info and see what else is loaded? The page-info dialog in Seamonkey is more useful for this than the default one in Firefox, it has extra categories/tabs. Do any of you folks who cc'd yourselves also see this, or did it just sound like an interesting bug? Have you tried another browser such as IE or Opera? Do they show mixed?

(In reply to comment #3) > Of course I can't tell for sure how the HTML is structured, but in both Firefox > logs we load the non-ssl > http://students.bankhapoalim.co.il/cgi-bin/inetcgi/student/exitGame.jsp and > images/scripts/css referenced from that file. > > Is this loaded in a frame, and therefore FF1.5 is fixing a 1.0.x bug? Or is > that just something that was loaded while you're exiting and the problem is > elsewhere. this page is where i'm taken when i logout of the account, because i have a student account. when i logout, in any of the four browsers, i get an alert: "although this page is encrypted, the information you entered will be sent over an unencrypted connection and can easily read by a third party..." (i'm back translating from hebrew to english here, because i'm using my localized version). > While you're on the mixed page could you look in Page Info and see > what else is loaded? The page-info dialog in Seamonkey is more useful for this > than the default one in Firefox, it has extra categories/tabs. > links: all links are javascipt: links. images: all images are loaded from https: urls. forms: one form, submitted to an https: url. framse: 7 frames, all but one are javascript: frames. the one that isn't is a https: frame. in any of the other branches in the Privacy tab there isn't any non-https url. > Do any of you folks who cc'd yourselves also see this, or did it just sound > like an interesting bug? > this problem was first noticed by one of our users (i head the hebrew l10n team) on firefox 1.5b2, and others verified, so others see this as well. > Have you tried another browser such as IE or Opera? Do they show mixed? IE6 doesn't report any problem. i don't have opera installed.

Confirming, based on the reports and based on the last comment, which seems to indicate this is a real bug.

Please note that while this might not be a security problem per se, it's certainly a big problem with the perception of security, and as such, I think efforts should be made so that it is fixed for the 1.5 release. As it is now we have two options: 1. Tell people to ignore the "broken lock" UI, assuring them that the site is actually secure (very bad idea, because users will then ignore it also when they shouldn't). 2. Tell people not to use Firefox on this site. Once again, not a good idea. Bank Hapoalim is the largest bank in Israel, with hundreds of thousands (perhaps even millions) of customers.

requesting blocking aviary2, 1.8rc1

tracking back nightlies, the bug seems to appear on the trunk between 2-9-2005 and 3-9-2005. the link to bonsai is: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-02+07%3A19&maxdate=2005-09-03+06%3A54&cvsroot=%2Fcvsroot

Could the regression have been caused by the change in bug 298064? I don't understand the code involved so I'm just making a wild guess.

(In reply to comment #9) > Could the regression have been caused by the change in bug 298064? I don't > understand the code involved so I'm just making a wild guess. > CC peterv based on that assumption

Does the page use document.open/document.write? Bug 298064 did change which document we use to propagate security info when using document.open(). We really need a (simple) testcase to figure this out.

this is the main frame of the account page, usually displaying the account info. as you can see, it has several document.write()'s. i removed any account info, as much as i could find. tell me if i missed something. it's not very simplified, but it's not too big either. i just didn't know what will interest you, as javascript often access the DOM. this is just one option for this frame - the default frame you get when you access the account. as you select to get other type of information, this frame is replaced to show that info.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051025 Firefox/1.5 I don't see a broken lock icon when I load the attachment in comment 12.

I tried to contact the bank about this problem a few days ago, but their contact form is broken and webmaster@ bounces.

(In reply to comment #14) > I tried to contact the bank about this problem a few days ago, but their > contact form is broken and webmaster@ bounces. I just tried the contact form at http://www.bankhapoalim.com/wps/portal/!ut/p/_.cmd/cs/ce/7_0_A/s./7_0_DR/_s.7_0_A/7_0_DR?displayJsp=intView1.jsp&area=a18 (with test data) and it seems to work (I got a "thank you" page). Perhaps you should try again?

It worked once I rewrote my comment to not use parentheses or hyphens.

or apostrophes.

cannot reproduce with simplified testcase on latest branch build on windowsXP. If it's the case that we're saying it's less secure than it is, that's not as horrible as the other way around.

(In reply to comment #14) > I tried to contact the bank about this problem a few days ago, but their > contact form is broken and webmaster@ bounces. > they also have a phone number. from abroad, it would be +972-3-6501155. they didn't say what the hours are, but israel is UTC+2

(In reply to comment #18) > cannot reproduce with simplified testcase on latest branch build on windowsXP. > If it's the case that we're saying it's less secure than it is, that's not as > horrible as the other way around. > i can upload a more complete frameset, but i don't think i can make bugzilla serve them all together in one page.

(In reply to comment #20) > i can upload a more complete frameset, but i don't think i can make bugzilla > serve them all together in one page. You can upload the individual frames, then change the frameset itself to use the bugzilla URLs of the frames you uploaded, and then upload it. Of course, if the frameset builds the URLs of the different frames dynamically, things might get more complicated.

(In reply to comment #21) > (In reply to comment #20) > > i can upload a more complete frameset, but i don't think i can make bugzilla > > serve them all together in one page. > > You can upload the individual frames, then change the frameset itself to use > the bugzilla URLs of the frames you uploaded, and then upload it. Of course, if > the frameset builds the URLs of the different frames dynamically, things might > get more complicated. > i thought about it, but the frameset also calls two javascript files, and the .js extension will disappear in bugzilla. i'll try uploading just the html files, and see if it helps.

Hmmm, I went trough the attached log and noticed this: 0[2745e0]: http request [ 0[2745e0]: GET /newimages/Internet_banner.gif HTTP/1.1 0[2745e0]: Host: newsletter.bankhapoalim.co.il 0[2745e0]: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; he; rv:1.8b5) Gecko/20051006 Firefox/1.4.1 0[2745e0]: Accept: image/png,*/*;q=0.5 0[2745e0]: Accept-Language: he-il,he;q=0.8,en-us;q=0.5,en;q=0.3 0[2745e0]: Accept-Encoding: gzip,deflate 0[2745e0]: Accept-Charset: windows-1255,utf-8;q=0.7,*;q=0.7 0[2745e0]: Keep-Alive: 300 0[2745e0]: Connection: keep-alive 0[2745e0]: Referer: https://login.bankhapoalim.co.il/cgi-bin/poalwwwc?******************************** 0[2745e0]: ] Notice that the host here is newsletter.bankhapoalim.co.il, unlike all other requests where it is login.bankhapoalim.co.il. Is it possible that the broken lock is shown because content from two different hosts is shown on the page (even if they're both HTTPS)?

sorry for all the spam...

(In reply to comment #23) Furthermore, there are references to newsletter.bankhapoalim.co.il in log-firefox1.5b2.txt and log-seamonkey1.0a.txt, but not in log-firefox1.0.7.txt. Tsahi, can you find the frame (or script) referencing https://newsletter.bankhapoalim.co.il/newimages/Internet_banner.gif, and see if it is different in 1.0.7 than in 1.5?

This testcase demonstrates a bug in Firefox that is a regression from Firefox 1.0.7 and causes the lock icon to become broken when it shouldn't. It might not be the same bug this bank's site hits, but I'm willing to bet that it is.

Testing with my testcase, this regressed on the branch between Sept 4 and Sept 5. This is consistent with the theory that my testcase tests the same bug and the regression was caused by the fix for bug 298064. (The fix went into the trunk a few days before it went into the branch, so I'm not contradicting comment 8 and comment 9.)

Brendan Eich and Blake Kaplan are working on fixing this bug.

this became a blocker today.

jst is going to back out the offending part of the patch from bug 298064 on the branch. I'm going to submit a fix in a couple of minutes to fix this on trunk and leave bug 298064 fixed (and compatible with other browsers).

This is a staight backout of the nsContentUtils.cpp changes (modulo a #include change that conflicts with other changes made on the branch) that caused this bug. This is the minimal change that needs to be backed out.

Johnny backed out the offending bit of bug 298064, marking fixed1.8.

Comment on attachment 201585 [details] [diff] [review] Trunk patch to fix >Index: content/base/public/nsContentUtils.h >+ * Get the document from the currently executing function's context (i.e., >+ * the static script global object). I'm not sure I follow this comment. In particular, it's not clear to me how this is different from GetDocumentFromContext(). I think the difference should be documented very very explicitly so people can tell which to use. Also, is anyone calling GetDocumentFromContext? I don't see it in this patch...

Comment on attachment 201585 [details] [diff] [review] Trunk patch to fix I forgot to diff nsHTMLDocument.cpp (the only user currently). I'll update the patch tomorrow.

i logged in www.bankhapoalim.co.il with a build from 2005-11-02 and everything looks fine.

The comments here are slightly better (suggestions on better comments are welcome) and I remembered to include nsHTMLDocument.cpp in the diff this time.

Comment on attachment 201688 [details] [diff] [review] Slightly better comments I still don't follow the difference. What is the difference between "the currently executing function" and "the JS context currently on the stack" in terms of origins? It would make the most sense to describe a situation in which those origins are different and explain which would give which origin...

Comment on attachment 201688 [details] [diff] [review] Slightly better comments r=jst with the comment changes...

Boris, I've added this comment above the two functions in nsContentUtils.h, does it help? /* * The two GetDocumentFrom* functions below allow a caller to get at a * document that is relevant to the currently executing script. * * GetDocumentFromCaller gets its document by looking at the last called * function and finding the document that the function itself relates to. * For example, consider two windows A and B in the same origin. B has a * function which does something that ends up needing the current document. * If a script in window A were to call B's function, GetDocumentFromCaller * would find that function (in B) and return B's document. * * GetDocumentFromContext gets its document by looking at the currently * executing context's global object and returning its document. Thus, * given the example above, GetDocumentFromCaller would see that the * currently executing script was in window A, and return A's document. */

Comment on attachment 201688 [details] [diff] [review] Slightly better comments Suggest using static vs. dynamic to capture static scope vs. dynamic (caller) scope. Caller (static, eek!) vs. Context is not clear -- these are not antonyms, and both reek of "dynamic scope". /be

Comment on attachment 201688 [details] [diff] [review] Slightly better comments sr=bzbarsky with the beefed-up comment, and a comment in nsHTMLDocument explaining why it's important to get the document from context, not caller. Also, might be worth doing a followup on the renaming brendan suggests.

Fix checked into trunk. I'll fix the function names seperately.

This is an attempt to backport this patch the 1.8 branch. I haven't yet compiled it.

Blake, the patch doesn't compile, it needs #include "nsPIDOMWindow" to compile (which should be done by replacing the #include of "nsIDOMWindowInternal"). Does anyone know for sure that this really is a problem on the branch? I don't see the problem with the testcases in this bug. Anyone have any input to verify this either way?

I believe this is required on the branch for 298064 (at least that's what BKap says over there)

Yeah, I put the patch here since that's where the trunk patch was, though that probably was a mistake. The question is whether or not we want to fix bug 298064 on the 1.8 branch (though I'm not sure how important that really is, now).

Comment on attachment 238242 [details] [diff] [review] 1.8 branch patch clearing the 1.8.1 patch flag since this is not ready to go..

Comment on attachment 238242 [details] [diff] [review] 1.8 branch patch r+sr=jst

Moving blocking flags from bug 298064 to this one. Please ask for approval for patches on the appropriate branches. Thanks!

Clearing the blocking1.8.1.1 flag... please correct me if I'm wrong in my understanding that this is already fixed on that branch.

Comment on attachment 238242 [details] [diff] [review] 1.8 branch patch Approved for 1.8.0 branch, a=jay for drivers. Please land this asap!

(In reply to comment #54) > Clearing the blocking1.8.1.1 flag... please correct me if I'm wrong in my > understanding that this is already fixed on that branch. Actually, it is not fixed on either branch. Basically, the regression from bug 298064 that this bug is about was fixed on trunk and both branches, but the fix for bug 298064 was partly backed out to do that. Attachment 238242 [details] [diff] restores the fix for bug 298064, and we want it on both branches. Maybe attachment 238242 [details] [diff] [review] should have been in bug 298064 so we don't confuse the fixed status of both bugs, but things are already confused enough as it is, so I'd rather leave it here and when it's checked in we'll mark bug 298064 as fixed on the branch too.

Comment on attachment 238242 [details] [diff] [review] 1.8 branch patch Approved for 1.8.0 and 1.8.1 branches, a=jay for drivers. Thanks peterv for the clarification. Please land asap.

Checked in attachment 238242 [details] [diff] [review] on branches.

QA: when verifying, please use testcases in both this bug and in bug 298064 (attachment 194172 [details]) and then also mark bug 298064 verified on the branches. Thanks.

Looks like juanb already verified bug 298064 on the branches. Verified FIXED using Jesse's "guess at simplified testcase" with the following builds: * Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9pre) Gecko/20061123 Firefox/1.5.0.9pre * Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1pre) Gecko/20061123 BonEcho/2.0.0.1pre

I can't seem to reproduce this bug on trunk even if I go back to GetDocumentFromCaller(), using the testcases attached here. Am I missing something?

maybe because it is VERIFIED FIXED?

Which part of "even if I go back to GetDocumentFromCaller()" didn't make sense?