Build: 02-18 trunk build By loading the page: http://www.ccidnet.com - this is a very popular Simplified Chinese web site, when load the page there will be a pop-up window, the machine CPU will up to 100% and then browser will hang. (See some comments in bug 126264). IE doesn't has same problem.

Please reassign in case the contact persons are wrong.

Setting default owner and QA -

This bug evolved out of bug 126264. Here are the findings made there: ------- Additional Comment #10 From Phil Schwartau 2002-02-19 11:27 ------- OK, the problem at that website is pretty bad. In the debug console, I see hundreds and hundreds of "WEBSHELL"'s being created. If I had let it continue, it would have gone over 1,000. My CPU does not go up to 100%, but the load just keeps going and going and eventually uses up all my virtual memory - I will attach a WinNT stack trace of manual interrupts I did during page load. ------- Additional Comment #11 From Phil Schwartau 2002-02-19 11:29 ------- Created an attachment (id=70349) WinNT stack trace of manual interrupts during page load ------- Additional Comment #12 From Yuying Long 2002-02-19 11:34 ------- Just for info.: IE will never has the problem that we had with those kind of pages. ------- Additional Comment #13 From Phil Schwartau 2002-02-19 11:39 ------- We have to decide what to do: 1. Morph this bug into another one for the new site? 2. Close this one as WORKSFORME and open a new one for the new site? I can't see any Plug-ins at the new site. In IE6 we see a little floating box that moves all over the page. I'll bet that's the problem. Somehow Mozilla is trying to create a new webshell for each new position of the floating box? Just a guess. The JS file for it is: view-source:http://www.ccidnet.com/image/java/ccidnet_floating01.js document.write("<DIV ID='fly' STYLE='position:absolute;width:80;height:80;visibility: visible'> <iframe width=80 height=80 noresize scrolling=No frameborder=0 marginheight=0 marginwidth=0 src=http://www.cnnet.com.cn/servlets/ad?Pool=home_floating> </iframe> </DIV> <script SRC='http://www.ccidnet.com/image/java/ccidnet_floating.js'> </script>"); ------- Additional Comment #14 From Phil Schwartau 2002-02-19 11:43 ------- Created an attachment (id=70355) ccidnet_floating.js ------- Additional Comment #15 From Phil Schwartau 2002-02-19 11:49 ------- Suspect function: function movechip(chipname) Suspect line: chip.timer1=setTimeout("movechip('"+chip.named+"')",140); This is neither JS Engine nor Plug-ins. Whether we keep this bug open, or file a new one, we need to find out: why does Mozilla create more than a thousand webshells when loading http://www.ccidnet.com ? Perhaps Embedding:Docshell component could tell ??? ------- Additional Comment #16 From Phil Schwartau 2002-02-19 11:52 ------- I would vote for closing this bug as WORKSFORME and opening a new one. That way, in case the problem comes back at the original site (i.e. the site changes its HTML back to what it was), we can reopen this bug and try to solve the problem. What does everybody think? ------- Additional Comment #17 From Yuying Long 2002-02-19 11:56 ------- I agree to open another bug(JavaScript?) for the new web site and mark this one here for works for me now. ------- Additional Comment #18 From Phil Schwartau 2002-02-19 12:00 ------- OK, note this also: There's another suspect file at the site: view-source:http://www.ccidnet.com/image/java/ccidnet_xl.js Just look at this time-bomb inside the file: function MM_reloadPage(init) { //reloads the window if Nav4 resized if (init==true) with (navigator) { if ((appName=="Netscape")&&(parseInt(appVersion)==4)) { document.MM_pgW=innerWidth; document.MM_pgH=innerHeight; onresize=MM_reloadPage; } } else if (innerWidth!=document.MM_pgW || innerHeight!=document.MM_pgH) location.reload(); } MM_reloadPage(true) Might this be causing an infinite loop of page loads - ? ------- Additional Comment #19 From Phil Schwartau 2002-02-19 12:03 ------- Yuying: the new bug should not be opened against JavaScript. There is nothing going wrong in the JavaScript Engine here. Based on what I have found, I would assign it to Embedding:Docshell to see if they can find out why so many webshells are being generated.

Bug 112491 has the same problem, endless webshells till hang or crash: http://www.bombardier.com/de/1_0/1_1/1_1_0.html ...copying over keywords from other bug Rephrasing summary from: Loading the page above will cause browser hang

*** Bug 118491 has been marked as a duplicate of this bug. ***

This has to be the most evil webpage I've ever seen! Can someone tell me if associative arrays like this are meant to work? document.images["but"+i].src=butover[i].src; Or referencing elements by their id, like ccidnet_xl.js does for layer called "dangdang" it writes into the document: document.dangdang.top=pageYOffset+window.innerHeight-imgheight

Argh this site is horrible!! After much searching (battling broken sniffers, iframes inside iframes inside iframes, document.writes everywhere, zillions of adverts, timers & codepages), I think it boils down to this URL here: http://www.ccidnet.com/road.html It generates a random number between 1 & 4 and stuffs in up to 4 different iframes adverts depending on the value. To take one example: http://www.cnnet.com.cn/servlets/ad?Pool=home_bur Here is the source for that URL: <!-- begin dynamic banner insert --> <!-- ad management by Ad Juggler 4.0 http://www.aj4.com --> <script LANGUAGE=JavaScript> <!-- document.write('<iframe width=0 height=0 noresize scrolling=No frameborder=0 marginheight=0 marginwidth=0 src="http://www.ccidnet.com/road.html"></iframe>'); //--> </SCRIPT> <embed src="http://www.ccidnet.com/image/468.swf" quality=high pluginspage=" http://www.macromedia.com/shockwave/download/index.cgi?P1_Prod_Version= ShockwaveFlash" type="application/x-shockwave-flash" width="468" height="60"></embed> <!-- Copyright 1999-2000 ThruPort Technologies http://www.thruport.com --> <!-- end dynamic banner insert --> So the first thing the advert does is run JS to insert another IFRAME pointing to road.html! In other words this is an recursive loop of IFRAMES. road.html inserts an IFRAME containing an advert which inserts an IFRAME containing road.html and so on. This is very disgusting code and I have no idea why it works in IE. Perhaps IE has some kind of limiter on how many IFRAMEs it will allow before quitting. I would be tempted to mark this WONTFIX, but I will wait for peoples opinion. It would be nice to be able to cap the number of nested IFRAMEs to prevent this abuse and for security reasons but I don't know how easy it would be to add. If someone knows of a non-intrusive way to implement this please say so.

cc'ing jst, jkeiser, Boris for their opinions on this -

There's a cap already in mozilla for how deep frames/iframes can nest, IIRC it's 25.

I found the code and bug 8065. http://lxr.mozilla.org/seamonkey/source/content/base/src/nsFrameLoader.cpp#295

Correction: http://lxr.mozilla.org/seamonkey/source/layout/html/document/src/nsFrameFrame.cpp#972 The other code is commented out. I'm working on a patch which lowers the limit. 25 seems pretty high.

This patch slashes MAX_DEPTH_CONTENT_FRAMES down from 25 to 8 and adds a warning when the limit is reached. With the limit in place, http://www.ccidnet.com/ just loads with 490 webshells and 100Mb memory consumption! 8 still seems a more than reasonable restriction on the number of nested *content* frames. If people feel it is too high or too low I can adjust it, but bear in mind that this sample page will not load with any higher number. It may not even load at 8 on some operating systems with less system resources, such as Win95/98/Me & Mac OS 9 etc. Reviews please.

This code has been moving around a few times lately (due to bug 52334 not sticking to the tree). When this is checked in, we should make this change either in nsFrameLoader.cpp or nsFrameFrame.cpp, depending on what code is used at the time. sr=jst for this change to either file.

Looking for an r= on the patch please.

Comment on attachment 77835 [details] [diff] [review] Patch reduces maximum limit for nested frames r=fabian

Nominate for adt1.0.0 Risk summary - low to moderate. Patch is simple and obvious, but there could be a website out there that needs 8 nested frames which won't work now. Justification for 1.0 - stops runaway sites earlier. Sites such as http://www.ccidnet.com that recursively create frames are less likely to take down Mozilla than with the old limit.

Actually, the real fix for thsi would be to add proper recursion detection code to the [i]frame loading code instead of limiting the nesting depth. Doing both is probably the ideal solution. Do we want to give that a shot for mozilla1.0? Shouldn't be hard to code up...

How would you see this code working? Is it ok to modify nsHTMLFrameInnerFrame to count how many parents use the same URI and halt after some arbitrary limit, e.g. 3? This might stop recursion sooner. Obviously comparing URIs isn't the ideal solution, but it's the only way to catch this kind of site where road.html is generating random content each time.

IMO if a frame ends up including itself (directly or indirectly) we should stop it right there. I don't know what IE does, but that's the only thing that makes sense to me. Comparing URI's seems like the obvious way to detect this, or do we have other better ways?

To recieve ADT approval, pls let us know, if the discussion between adam and jst needs to be resolved, or is this patch ready to go as is?

Jaime: the problems are orthogonal. Adam's fix can go in the tree IMO. JST: comparing URIs will work. One legit problem is when there is a form POST that results in a page with iframe or frame, and the URI is the same as one of the parents. That is some really strange code though. However ... if the reason we want to stop the recursion is malicious intent, any ol' coder can get around it by making a simple script at http://www.evilcode.com that loads two frames with http://www.evilcode.com/script?1, that each load two frames with http://www.evilcode.com/script?2, and so on, and so on ... simple programming hack. Adam's code will stop that though. Adam's code will also stop accidental recursion so that a webmaster can fix their site. Adam: I personally think 8 is fine. Only insane people nest 8 frames. And we all know there are no insane people writing web pages.

Jaime, This patch is ready for adt approval. The other issue can be dealt with seperately. Johnny, the problem with the sample URL is that pages don't just include themselves. The pattern is somewhat like this: road.html foo1.html road.html foo1.html .. etc .. foo2.html foo2.html .. etc .. foo3.html .. etc ... So references to road.html are sandwiched between references to other URLs. To catch this kind of behaviour we would have to walk back through the parents, counting matching URLs. Obviously a URL could be accompanied by postdata, cookies, js etc. which means matching URLs may have different content. The safeguard to this is count the matches and only reject after say 3 identical URLs are found. Redirects might also screw around with this code. This is no solution to malicious attacks, but it could stop runaway idiot code.

Fine with me, let's either leave this bug open after landing this fix or file a new bug on better recursion protection wrt frames.

adt1.0.0+ (on ADT's behalf) approval for checkin into 1.0. Pls check this one in as soon as you have a= from drivers.

Comment on attachment 77835 [details] [diff] [review] Patch reduces maximum limit for nested frames a=rjesup@wgate.com for drivers. If this misses the branch, please nominate for checkin to the branch.

Fix checkinto trunk still waiting for 1.0 branch approval.

Fix checked into branch. I will raise a new bug on doing URL comparison.

Raised bug 136580 to examine ways to catch recursive code earlier.

It seems that there is an evangelism issue here. I happened to have filed Bug 132582 last month. It is this exact same bug. There are a couple of things that concern me. 1. With IE 5.5, I saw CPU consumption fluctuating between 65-95%. While it did not crash, the program became eventually unusable after a few minutes. 2. With Mozilla trunk build, I saw the CPU consumption going up to 100%. Then crash within 30 seconds or so when you do try to do anything with Mozilla. The fix here would avoid a hang but probably would not prevent CPU consumption going up, would it? Please explain what this fix would prevent and would not prevent. Are we just trying to be on the par with IE, which would also crash or become unusable in 1-2 minutes? My sense is that this site needs to be evangelized not to code in a way that forces browers to become more or less frozen. I will keep the other bug open and contact the web site unless I here otherwise.

adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword with verified1.0.0.

Current trunk debug build shows the number of webshells reaching 100 and layout giving up. Is it a regression or this is how it should be?

That's how it should be. Show me a site with more than 100 frames and I'll show you a site with too much time on its hands :)

Mozilla currently hangs on the site in question, though. IE and Netscape 4.x seem to handle it in stride. Should a new bug be opened??

And I am talking about the original url. I did not look at it... Does it have 100 frames or this is a recursion problem?

Recursion.