Bug 84352

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

As of now konqueror implements frame searching as described in
    http://www.w3.org/TR/html401/appendix/notes.html#notes-frames
    sec. B.8
which was the "common practice".

So, should we just ditch point 3) altogether:
   3- If no such frame was found in (2), apply step 2 to each window, in a
   front-to-back ordering. Stop as soon as you encounter a frame with exactly the
   same name.
(which is implemented in KonqMainWindow::slotOpenURLRequest)
or rather restrict the search to frames with a matching url.host()?

I think the frame lookup by konqueror in KonqView::hostExtension() should be moved out of konqueror and into the html part and then made subject to domain restrictions. I'm working on a patch to make that possible.

Created attachment 6539 [details]
patch

This patch adds domain checks for cross-frame part loading. it breaks
the case when the frame it should load is not reachable from the current
root of the frame tree. I'm not sure if that is relevant or not. 

Another possibility would be to only implement this stricter checking for
https urls, but its probably wise to check what the other browser vendors
came up with.

See also http://bugzilla.mozilla.org/show_bug.cgi?id=246448

It looks like the deutsch bank site that the heise article is linking to is now returning a frameless page.

http://bugzilla.mozilla.org/show_bug.cgi?id=246448#c37 contains a useful testcase

Created attachment 6550 [details]
patch

Patch #6550 implements comment #3. 

It tries to implement the behavior as described for the testcase linked to in comment #7 but I don't quite get that because htmlDocument().domain() does not
give me what I would expect to get. E.g. if a document containing a frameset has http://content.localhost/framespoof/contentframe.html as URL then I had expected the domain for that document to be "content.localhost", but instead it seems to get the domain from its parent frameset.

Created attachment 6586 [details]
khtml-2.patch

Updated khtml patch (Replaces khtml.patch from 6550)

I could only test Dirk's patch for now...
It looks simple and predictible enough to be safely applied on BRANCH, but maybe lacks flexibility... 

I found there is still a gap at least for form submissions in this patch: they use to go directly through openURLRequest(), letting it find a suitable frame. 
So http://bugzilla.mozilla.org/attachment.cgi?id=150586&action=view can still spoof us.
Also, the patch doesn't let script without target be executed.
Incrementally, it would at least need something like:

--- khtml_part.cpp	2004-07-03 19:49:00.000000000 +0100
+++ khtml_part.cpp.n	2004-07-03 18:14:35.000000000 +0100
@@ -868,6 +868,9 @@
 {
   destpart = this;

+  if (target.isEmpty())
+     return true;
+
   QString trg = target.lower();

   if (target == "_top") {
@@ -4350,6 +4353,9 @@
   }
   else
   {
+    KHTMLPart* dest;
+    if ( !allowCrossFrameTarget( args.frameName, dest ) )
+      args.frameName = "_blank";
     emit d->m_extension->openURLRequest( u, args );
   }
 }

Re #11: 6550 and 6586 address these issues by concentrating the checking in findFrame/findFrameParent

6539 also seems to break the case where a frame in another window is referenced, while 6550/6586 will handle that subject to domain restrictions.

Commited to CVS HEAD since it fixes the reported vulnerability, but the domains reported by the parts still seem to be wrong.

Ugh, in CVS the test on http://secunia.com/advisories/11978/ still fails.

That's because it runs in the
   if (!htmlDocument().isNull()) {

check which returns true. If I return false there the normal frame navigation on msdn.com stops working. The part with the frameset doesn't seem to have a htmlDocument at all???

The mystery in #14 is solved by changing the check into
   if (htmlDocument().isNull()) {

Boolean logic is always tricky :-}

I got it right, that the first and third patch are needed? Are these patches simple to backport or do I have to care for something?

Backported patches send by mail

mmh, I made some comments on the related kfm-devel thread some time ago and CC-ed here, but I used the wrong email address so it never reached here.

Waldo, there seem to be some problems left concerning the frame that is chosen.
I found one good testcase today:

http://ktown.kde.org/~coolo/regression/output/

1) open in a tab
2) duplicate the tab
3) go back to first tab and select one testcase
=> will open in the_other_ tab.

Thanks, fixed now.

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

*** Bug has been marked as fixed ***.

So how was it fixed in the end? And when? 
As of Konqueror 3.3, JavaScript open() function is still utterly broken... it breaks my e-banking site into pieces (literally). See testcase at http://barium.mine.nu/~divide/brokenjs/

It's OK now, thanks for your great work.